From: Sylvain Munaut <tnt@246tNt•com>
To: Domen Puncer <domen@coderock•org>
Cc: spi-devel-general@lists•sourceforge.net,
Dragos Carp <dragos.carp@toptica•com>,
David Brownell <dbrownell@users•sourceforge.net>,
linuxppc-embedded@ozlabs•org
Subject: Re: [PATCH] mpc52xx_psc_spi: fix it for CONFIG_PPC_MERGE
Date: Wed, 16 May 2007 10:19:35 +0200 [thread overview]
Message-ID: <464ABE97.7090603@246tNt.com> (raw)
In-Reply-To: <20070516073707.GD9667@nd47.coderock.org>
Well, this comment is not about the patch but about the driver it self,
I didn't see it before today.
So here's a few things I see from a quick glance at the code :
- Chaning port_config in the driver is just wrong ... you should _not_
do that. That should have been setup by the bootloader or at worse in
the platform setup code.
- You do read/write/modify operation on CDM shared register
(clk_enables) from a driver, you should have added something in common
52xx code to do theses with proper locking.
- MPC52xx_PA(MPC52xx_PSCx_OFFSET(...)) ??? You should get that from the
resource of the platform_device. This macro is just there for early
console stuff.
- You can get f_system from the device tree instead of just assuming
it's 512 MHz. It probably need to be done the same way it's done to find
ipb_freq.
- Would have been nice to be able to somehow configure MCLK rather than
#define it
- I hope to remove all arch/ppc stuff by 2.6.23 if I can make the
cuimage stuff work in arch/powerpc so just including the platform code
stuff for 1 kernel version ...
Sylvain
next prev parent reply other threads:[~2007-05-16 8:19 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-05-16 7:37 [PATCH] mpc52xx_psc_spi: fix it for CONFIG_PPC_MERGE Domen Puncer
2007-05-16 8:19 ` Sylvain Munaut [this message]
2007-05-16 16:11 ` David Brownell
2007-05-16 16:34 ` Sylvain Munaut
2007-05-18 7:44 ` Dragos Carp
2007-05-25 8:43 ` [RFC 1/3] " Domen Puncer
2007-05-25 14:50 ` Sylvain Munaut
2007-05-25 17:02 ` Grant Likely
2007-05-25 8:45 ` [RFC 2/3] " Domen Puncer
2007-05-25 8:47 ` [RFC 3/3] " Domen Puncer
2007-05-25 16:34 ` David Brownell
2007-05-25 18:00 ` Domen Puncer
2007-05-21 7:31 ` Dragos Carp
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=464ABE97.7090603@246tNt.com \
--to=tnt@246tnt$(echo .)com \
--cc=dbrownell@users$(echo .)sourceforge.net \
--cc=domen@coderock$(echo .)org \
--cc=dragos.carp@toptica$(echo .)com \
--cc=linuxppc-embedded@ozlabs$(echo .)org \
--cc=spi-devel-general@lists$(echo .)sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox