linux-kernel - RE: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM3PR04MB306D5C86AED120D15C4D46A80E70@AM3PR04MB306.eurprd04.prod.outlook.com>
Date:   Wed, 17 May 2017 06:01:37 +0000
From:   "A.S. Dong" <aisheng.dong@....com>
To:     Nikita Yushchenko <nikita.yoush@...entembedded.com>,
        Dong Aisheng <dongas86@...il.com>
CC:     "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        Andy Duan <fugang.duan@....com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "Y.B. Lu" <yangbo.lu@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stefan@...er.ch" <stefan@...er.ch>,
        Mingkai Hu <mingkai.hu@....com>,
        "jslaby@...e.com" <jslaby@...e.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register
 support

> -----Original Message-----
> From: Nikita Yushchenko [mailto:nikita.yoush@...entembedded.com]
> Sent: Wednesday, May 17, 2017 1:44 PM
> To: Dong Aisheng
> Cc: A.S. Dong; linux-serial@...r.kernel.org; Andy Duan;
> gregkh@...uxfoundation.org; Y.B. Lu; linux-kernel@...r.kernel.org;
> stefan@...er.ch; Mingkai Hu; jslaby@...e.com; linux-arm-
> kernel@...ts.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