From: Sylvain Munaut <tnt@246tNt•com>
To: David Brownell <david-b@pacbell•net>
Cc: spi-devel-general@lists•sourceforge.net,
Dragos Carp <dragos.carp@toptica•com>,
Domen Puncer <domen@coderock•org>,
linuxppc-embedded@ozlabs•org
Subject: Re: [PATCH] mpc52xx_psc_spi: fix it for CONFIG_PPC_MERGE
Date: Wed, 16 May 2007 18:34:38 +0200 [thread overview]
Message-ID: <464B329E.8030600@246tNt.com> (raw)
In-Reply-To: <200705160911.55867.david-b@pacbell.net>
David Brownell wrote:
> On Wednesday 16 May 2007, Sylvain Munaut wrote:
>
>> Well, this comment is not about the patch but about the driver it self,
>> I didn't see it before today.
>>
>
> It merged earlier in the 2.6.22 cycle. If you don't have criticisms
> about the patch itself, I'll forward it for merging after I get at
> least an ack from Dragos.
>
Yes, I saw when looking at the spi-devl archive. Would have been nice if the
author though of cc-ing the ppc-embedded list ;)
The patch looks ok to me (and needed actually since as Domen pointed
out, 52xx
has been replaced by 5200 in the device tree).
And cell-index has been added to know the psc id without dirty tricks.
>> - 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.
>>
>
> That PPC_MERGE stuff does look messy.
>
Yes, trying to support both in a driver is really not pretty.
Once we can finally get rid of it I'll submit a patch to clear that out.
>> - 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.
>> - 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
>>
>
> Best to use <linux/clk.h> for all of those, but it seems powerpc/ppc
> don't support those interfaces yet ... is there maybe a plan for
> resolving that issue?
>
Mmm, I wasn't aware of that interface, I'll look into that. Thanks.
Sylvain
next prev parent reply other threads:[~2007-05-16 16:34 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
2007-05-16 16:11 ` David Brownell
2007-05-16 16:34 ` Sylvain Munaut [this message]
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=464B329E.8030600@246tNt.com \
--to=tnt@246tnt$(echo .)com \
--cc=david-b@pacbell$(echo .)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