lkml.org 
[lkml]   [2017]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
    Date
    > -----Original Message-----
    > From: Nikita Yushchenko [mailto:nikita.yoush@cogentembedded.com]
    > Sent: Wednesday, May 17, 2017 1:44 PM
    > To: Dong Aisheng
    > Cc: A.S. Dong; linux-serial@vger.kernel.org; Andy Duan;
    > gregkh@linuxfoundation.org; Y.B. Lu; linux-kernel@vger.kernel.org;
    > stefan@agner.ch; Mingkai Hu; jslaby@suse.com; linux-arm-
    > kernel@lists.infradead.org
    > Subject: Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit
    > register support
    >
    > >>> @@ -2000,6 +2007,7 @@ static int lpuart_probe(struct platform_device
    > *pdev)
    > >>> }
    > >>> sport->port.line = ret;
    > >>> sport->lpuart32 = sdata->is_32;
    > >>> + lpuart_is_be = sdata->is_be;
    > >>
    > >> Setting a global variable in per-device routine is quite bad design.
    > >>
    > >
    > > There is a reason for that we don't want to change the exist
    > > lpuart32_read[write] API which is widely used in driver.
    > > Making a global lpuart_is_be is the simplest way to do it.
    > >
    > > Any strong blocking reason?
    >
    > Code should be consistent.
    >

    Yes.

    > There is no good reason to have sport->lpuart32 inside sport, but
    > lpuart_is_be outside of it. Both these values describe properties of
    > particular device, and thus should be in per-device structure.
    >

    That's for special case, normally we wouldn't do that.

    > If that implies adding sport arg to lpuart32_(read|write), just do that.

    There's another reason that we have to deal with earlycon which is
    executed much early before driver probe.

    And I need specificly align the endian data.
    e.g.
    static int __init lpuart32_early_console_setup(struct earlycon_device *device,
    const char *opt)
    {
    if (!device->port.membase)
    return -ENODEV;

    lpuart_is_be = true;
    device->con->write = lpuart32_early_write;
    return 0;
    }

    static int __init lpuart32_imx_early_console_setup(struct earlycon_device *device,
    const char *opt)
    {
    if (!device->port.membase)
    return -ENODEV;

    lpuart_is_be = false;
    device->port.membase += IMX_REG_OFF;
    device->con->write = lpuart32_early_write;

    return 0;
    }

    Regards
    Dong Aisheng

    \
     
     \ /
      Last update: 2017-05-17 08:02    [W:4.427 / U:0.192 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site