Re: [Patch] mpc5200b: improve baud rate calculation (reach high baud rates, better accuracy)

[Grant Likely]

On Tue, Mar 2, 2010 at 1:09 AM, Albrecht Dreß <albrecht.dr...@arcor.de> wrote:
>> > +   /* Check only once if we are running on a mpc5200b or not */
>> > +   if (is_mpc5200b == -1) {
>> > +           struct device_node *np;
>> > +
>> > +           np = of_find_compatible_node(NULL, NULL, "fsl,mpc5200b-immr");
>>
>> This should be handled using a new compatible-entry
>> "fsl,mpc5200b-psc-uart".
>
> I agree that this would be a lot cleaner, but it's also a lot more intrusive. 
>  CC'ing the device tree discussion list here... comments, please!!

fsl,mpc5200b-psc-uart is already in the compatible list for all
MPC500b boards currently in the kernel tree.

>> > +           if (np) {
>> > +                   is_mpc5200b = 1;
>> > +                   dev_dbg(&op->dev, "mpc5200b: using /4 prescaler\n");
>>
>> Does this message respect the fallback case?
>
> See comment above...
>
>> You could also have a set_divisor-function for 5200 and 5200B and set it
>> here
>> in the function struct (one reason less for the static ;))
>
> Hmmm, but then I would need a 'static struct psc_ops mpc5200b_psc_ops', where 
> only two functions differ from the generic 52xx struct as it is implemented 
> now.  Using the static int needs less space.  However, in combination with 
> the new compatible entry, it would of course make sense.
>
> Again, any insight from the device tree gurus would be appreciated!

Wolfram is correct, you should set the correct divisor function in the
ops structure.  Much clearer code that way.

g.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev