[PATCH] ns16550: switch platform_data to drivers private data

Antony Pavlov antonynpavlov at gmail.com
Fri Aug 5 03:52:15 EDT 2011


On 5 August 2011 10:32, Sascha Hauer <s.hauer at pengutronix.de> wrote:

> On Fri, Aug 05, 2011 at 12:58:32AM +0400, Antony Pavlov wrote:
>> On 4 August 2011 23:37, Jean-Christophe PLAGNIOL-VILLARD
>> <plagnioj at jcrosoft.com> wrote:
>>
>> > --- a/drivers/serial/serial_ns16550.c
>> > +++ b/drivers/serial/serial_ns16550.c
>> > @@ -48,6 +48,55 @@
>> >
>> >  /*********** Private Functions **********************************/
>> >
>> > +/*
>> > + * We wrap our port structure around the generic console_device.
>> > + */
>> > +struct ns16550_uart_port {
>> > +       void __iomem *base;
>> > +       uint32_t shift;
>> > +       uint32_t clock;
>> > +       uint32_t (*read)(void __iomem *base, uint8_t off);
>> > +       void (*write)(uint32_t val, void __iomem *base, uint8_t off);
>> > +
>> > +       struct console_device uart;
>>
>>           nice trick :)))
>>
>>           But why the name is 'uart', not 'cdev'?
>> > +};
>>
>> As a rule, structure declarations go to header files.
>
> And as another rule, locally used struct (and also defines) go to the C
> file.
The file drivers/serial/serial_ns16550.h has so many defines of such type ...

>>
>> > +
>> > +static uint32_t ns16550_readb(void __iomem *base, uint8_t off)
>> > +{
>> > +       return readb(base + off);
>>
>>  warning: pointer of type ‘void *’ used in arithmetic
>
> Honestly, I can find nothing wrong in void * arithmetic. The
> alternatives like casting to/from unsigned long are much worse.

unsigned long is bad. We have discussed this two weeks ago.
See http://lists.infradead.org/pipermail/barebox/2011-July/003957.html

But void * is not created for arithmetic.
On the other hand we must choose the lesser of two evils.

-- 
Best regards,
  Antony Pavlov



More information about the barebox mailing list