linux-kernel - Re: [PATCH 2/4] Add SUNIX Multi-I/O card device driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAK8P3a2yKZqB1CtYSSrhHQmRDdE-GpwdHMifgPB5Ledv-UJzWw@mail.gmail.com>
Date:   Mon, 11 Mar 2019 16:05:28 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Morris Ku <saumah@...il.com>
Cc:     gregkh <gregkh@...uxfoundation.org>, morris_ku@...ix.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] Add SUNIX Multi-I/O card device driver

On Fri, Mar 8, 2019 at 1:34 PM Morris Ku <saumah@...il.com> wrote:
>
> Driver for SUNIX Multi-I/O card.
> Based on driver/char/serial.c by Linus Torvalds, Theodore Ts'o.
>
> SUNIX serial card designed with SUNIX UART controller and
> compatible with 16C950 UART specification.
>
> Signed-off-by: Morris Ku <saumah@...il.com>

Hi Morris,

Thanks for your submission!

Enrico has already replied with the most important comments. Here
are just some more high-level thoughts from me:

* We want the functionality to be present in subsystem specific
  standalone drivers, i.e. drivers/tty/serial/, drivers/parport/, drivers/gpio
  etc, as Enrico said

* It looks like you may need some glue logic to tie those together for
  the multi-I/O cards. This is a bit tricky but doable. Many other multi-I/O
  cards simply appear as a set of PCI functions that are connected to
  a single slot. This is the ideal case, as you can simply have one
  driver per function and they do not have to be linked together at all.
  If this is not possible, what you may need to do here is to have a
  small driver in drivers/mfd/ that handles the PCI function and creates
  platform_device instances that the individual drivers can bind to.
  Have a look at the various files in drivers/mfd that come with a
  'struct pci_driver' instance to see what I mean.

* In any way, new drivers must probing according to our driver
  module if possible. This means you should have a 'pci_driver'
  structure listing the possible vendor/device ID combinations,
  and register that with 'module_pci_driver'. If you use mfd_cell
  to register sub-drivers, use 'module_platform_driver' to register
  the driver for those.

* do not implement your own ioctl handlers. If you need a custom
  interface for something hardware specific, split that out into
  a separate patch, so the base support can be reviewed
  independently.

* If the functions (in particular serial and parport) have a similar
  register layout to existing drivers, try to reuse the existing code
  and extend it, rather than duplicating the implementation.

       Arnd