linux-kernel - Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the number of xilinx ps uarts

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: <b5a032c8-db86-c16b-750f-10aa2a132248@xilinx.com>
Date:   Wed, 24 May 2017 18:09:11 +0200
From:   Michal Simek <michal.simek@...inx.com>
To:     Alan Cox <gnomes@...rguk.ukuu.org.uk>,
        Michal Simek <michal.simek@...inx.com>
CC:     Rob Herring <robh+dt@...nel.org>,
        Sam Povilus <kernel.development@...il.us>,
        <gregkh@...uxfoundation.org>, <jslaby@...e.com>,
        <soren.brinkmann@...inx.com>, <linux-serial@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] xilinx ps uart: Adding a kernel parameter for the
 number of xilinx ps uarts

On 24.5.2017 15:31, Alan Cox wrote:
>> I am not saying that config option is perfect solution. It is at least
>> aligned with 5 others serial drivers in the tree.
> 
> And the fact people keep doing hacks jutifies continuing to make a mess.
> Especialy as in this case it's entirely theoretical. Nobody has produced
> actual hardware that hits the limit. Nobody has filed a bug, nobody is
> impacted.

This is the reason why we are talking about it how to do it right.

With this ps uart it is not that easy because this is cadence RTL which
is not in public IP database but the same think is with uartlite.
Limit there is 16. If you really want that I can create that HW design
which will require more than 16 uartlites in one design.

> Creating extra CONFIG_ entries for junk like this is ridiculous, most of
> the others at least have the excuse of being old code.

No doubt about it. I am just trying to find out what's the way you are
suggesting.

> Generally it is better to call uart_register_driver from the probe method
> because that way you don't waste time and memory registering drivers for
> stuff that isn't even present however if you have no idea how many
> devices there might be then you still really need to pass a suitable limit
> and handle it internally dynamically allocating as needed.

Ok. Is there any problem if uart_register_driver is called for every
instance separately with nr=1? This driver has major 0, minor 0. Based
on comment major 0 is for dynamic node allocation. Not sure about minor
but it is easy to figured out if this should be 0 or 1, 2, 3, etc.

> If someone was hitting this in the real world and you posted a patch that
> just changed the constant to 8 or 16 or whatever was needed I wouldn't
> care too much, but adding CONFIG_ entries just makes stuff harder and
> harder to config and more and more impossible to keep generic.
> 
> I keep hearing that the ARM folks are trying to get one unified kernel.
> CONFIG_ options is not how to do that.

I have really not a problem with all of this. Just trying to understand
how to do it properly and cleanup the second driver which we use on
fpga. Last driver used by Xilinx is uart16550 where that old config
macro already exists.

Because at least now there is an issue in driver if you use serial
aliases (serial2 and up) which needs to be fixed.

Thanks,
Michal