Messages in this thread | | | Subject | Re: [PATCH] Respond:Add SUNIX-Multi-I/O card device driver | From | "Enrico Weigelt, metux IT consult" <> | Date | Wed, 13 Mar 2019 18:36:36 +0100 |
| |
On 13.03.19 14:31, Morris Ku wrote:
Hi,
> +why isn't that in ./drivers/tty/serial/sunix/ ? > + > driver support SUNIX Character Devices, > serial ports and parallel ports,so we suggest > that in /drivers/char.
Well, this seems to be a composite device. so it should be actually different drivers (initialized by one compound driver) - all of the subdevices I'm seeing here have their own subsystems, none of them being a plain chardev. Therefore it probably should go to drivers/mfd (multi function devices). If you split out the subdevice drivers (which I'd recommend), these should go to the corresponding subsystem directories (eg. drivers/tty/serial/ for the serial subdevice).
> +please use full name: SUNIX > + > > Ok, i'll fix in next verion.
Oh, by the way: SUNIX is the company name ? So, there's probably some device/product name. I'd put that into the config name, too.
Eg. if the device is called "FANCYIO", then the config symbol would be SUNIX_FANCYIO.
> +why exactly do you introduce driver-specific ioctls ? > + > ioctl functions support SUNIX specific serial,parellel and GPIO > ,need to use specific ioctol function to get related inforomation.
Which information, exactly, that aren't supported by the corresponding subsystems ?
To make it clear: the individual functions of this card should go into the corresponding subsystem. So, you'd have a serial driver, a parallel driver, a gpio driver - all living in the corresponding subsystems. NOT: one driver (more precisely: one chardev) to rule them all.
Note: in Linux we wanna use generic APIs where we can. So, if somebody wants to use a GPIOs, he goes by the GPIO subsystem - no matter which hardware it actually is - no hw specific devices/ioctl.
> +what is "ACS" > + > RS485 ACS Enable,Enable SUNIX UART Controller waiting transmit > data when receive data is going.
See struct uart_port and corresponding helpers (drivers/tty/serial), it already has infrastructure for that. It also does all the buffering stuff for you.
> SUNIX Multi-I/O card combaine serial ports,parallel ports and GPIO, > therefore, the define support SUNIX specific gpio interface.
See above: don't introduce your own specific interfaces - use the generic ones. Seriously, that's the way it's going on Linux.
> +> + char *dma_buf; > + > +why not void * ? > + > i am not sure what you mean ?
Is it really correct that the dma_buf has to be char* ? (and even w/o signed/unsigned attribute).
For opaque memory buffers, we usually use void*.
> +> +// snx_devtable.c > +> +extern PCI_BOARD snx_pci_board_conf[]; > +<snip> > + > +why all these extern functions ? > + > function definition in multiple .c files.
it's not a function, but an array of structs.
--mtx
-- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
| |