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 [email protected] -- +49-151-27565287
