| Message ID | 20180611115240.32606-16-ricardo.ribalda@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | None | expand |
Hi Ricardo, > Export serdev table to the module header, allowing module autoload via > udev/modprobe. > > Cc: Lino Sanfilippo <LinoSanfilippo@gmx.de> > Cc: David S. Miller <davem@davemloft.net> > Cc: Stefan Wahren <stefan.wahren@i2se.com> > Cc: Rob Herring <robh@kernel.org> > Cc: Johan Hovold <johan@kernel.org> > Cc: netdev@vger.kernel.org > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > drivers/net/ethernet/qualcomm/qca_uart.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c > index db6068cd7a1f..6d2ac6cae63f 100644 > --- a/drivers/net/ethernet/qualcomm/qca_uart.c > +++ b/drivers/net/ethernet/qualcomm/qca_uart.c > @@ -405,6 +405,12 @@ static void qca_uart_remove(struct serdev_device *serdev) > free_netdev(qca->net_dev); > } > > +static struct serdev_device_id qca_uart_serdev_id[] = { > + { QCAUART_DRV_NAME, }, > + {} > +}; > +MODULE_DEVICE_TABLE(serdev, qca_uart_serdev_id); > + > static struct serdev_device_driver qca_uart_driver = { > .probe = qca_uart_probe, > .remove = qca_uart_remove, > @@ -412,6 +418,7 @@ static struct serdev_device_driver qca_uart_driver = { > .name = QCAUART_DRV_NAME, > .of_match_table = of_match_ptr(qca_uart_of_match), > }, > + .id_table = qca_uart_serdev_id, > }; the commit message is misleading me. If I build something with ACPI or DT support, then modinfo will show all modalias information for ACPI and DT compatible strings. What else does udev/modprobe actually need? Is something broken with the modalias export? Regards Marcel
Hi Marcel, On Mon, Jun 11, 2018 at 3:01 PM Marcel Holtmann <marcel@holtmann.org> wrote: > > > the commit message is misleading me. If I build something with ACPI or DT support, then modinfo will show all modalias information for ACPI and DT compatible strings. What else does udev/modprobe actually need? Is something broken with the modalias export? The main purpose is to autoload drivers for devices that have been created via sysfs or another module. Eg1: We have a serial port on a standard computer that has connected a GPS module. Since it is something that is not in the ACPI nor the DT table the user will run echo serdev_gps > /sys/bus/serial/devices/serial0/new_device Eg2 module: https://github.com/ribalda/linux/blob/415bb3f0076c2b846ebe5409589b8e1e3004f55a/drivers/tty/serdev/test_platform.c Modprobe does not know what module to load for that device unless there is a matching MODULE_DEVICE_TABLE Today, we have the same functionality for i2c devices https://www.kernel.org/doc/Documentation/i2c/instantiating-devices I guess the commit message is really bad :), sorry about that. Any suggestions to improve it? Thanks! > > Regards > > Marcel >
Hi Marcel, >> the commit message is misleading me. If I build something with ACPI or DT support, then modinfo will show all modalias information for ACPI and DT compatible strings. What else does udev/modprobe actually need? Is something broken with the modalias export? > > The main purpose is to autoload drivers for devices that have been > created via sysfs or another module. > > Eg1: We have a serial port on a standard computer that has connected a > GPS module. Since it is something that is not in the ACPI nor the DT > table the user will run > > echo serdev_gps > /sys/bus/serial/devices/serial0/new_device > > Eg2 module: https://github.com/ribalda/linux/blob/415bb3f0076c2b846ebe5409589b8e1e3004f55a/drivers/tty/serdev/test_platform.c > > Modprobe does not know what module to load for that device unless > there is a matching MODULE_DEVICE_TABLE > Today, we have the same functionality for i2c devices > https://www.kernel.org/doc/Documentation/i2c/instantiating-devices but why does this have to be the driver name? I would rather say, create a generic string that describes the hardware and then use that. I think you also want the special handling, as from USB for example, that lets you reference an existing .compatible or ACPI ID as reference so that the driver_data is copied. Regards Marcel
Hi Marcel, On Mon, Jun 11, 2018 at 5:28 PM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Marcel, > > >> the commit message is misleading me. If I build something with ACPI or DT support, then modinfo will show all modalias information for ACPI and DT compatible strings. What else does udev/modprobe actually need? Is something broken with the modalias export? > > > > The main purpose is to autoload drivers for devices that have been > > created via sysfs or another module. > > > > Eg1: We have a serial port on a standard computer that has connected a > > GPS module. Since it is something that is not in the ACPI nor the DT > > table the user will run > > > > echo serdev_gps > /sys/bus/serial/devices/serial0/new_device > > > > Eg2 module: https://github.com/ribalda/linux/blob/415bb3f0076c2b846ebe5409589b8e1e3004f55a/drivers/tty/serdev/test_platform.c > > > > Modprobe does not know what module to load for that device unless > > there is a matching MODULE_DEVICE_TABLE > > Today, we have the same functionality for i2c devices > > https://www.kernel.org/doc/Documentation/i2c/instantiating-devices > > but why does this have to be the driver name? I would rather say, create a generic string that describes the hardware and then use that. I think you also want the special handling, as from USB for example, that lets you reference an existing .compatible or ACPI ID as reference so that the driver_data is copied. We can choose any name, but if there are no special variants (like the rave-sp driver) I would prefer using the module name. We can of course use more names, like the part number of the the device, but it is very convenient to also use the module name, and other subsystems also do that. If you want to add specific names for this device please let me know and I will add them to the list. You know much more about this module than myself. Thanks! > > Regards > > Marcel >
Hi Ricardo, >>>> the commit message is misleading me. If I build something with ACPI or DT support, then modinfo will show all modalias information for ACPI and DT compatible strings. What else does udev/modprobe actually need? Is something broken with the modalias export? >>> >>> The main purpose is to autoload drivers for devices that have been >>> created via sysfs or another module. >>> >>> Eg1: We have a serial port on a standard computer that has connected a >>> GPS module. Since it is something that is not in the ACPI nor the DT >>> table the user will run >>> >>> echo serdev_gps > /sys/bus/serial/devices/serial0/new_device >>> >>> Eg2 module: https://github.com/ribalda/linux/blob/415bb3f0076c2b846ebe5409589b8e1e3004f55a/drivers/tty/serdev/test_platform.c >>> >>> Modprobe does not know what module to load for that device unless >>> there is a matching MODULE_DEVICE_TABLE >>> Today, we have the same functionality for i2c devices >>> https://www.kernel.org/doc/Documentation/i2c/instantiating-devices >> >> but why does this have to be the driver name? I would rather say, create a generic string that describes the hardware and then use that. I think you also want the special handling, as from USB for example, that lets you reference an existing .compatible or ACPI ID as reference so that the driver_data is copied. > > We can choose any name, but if there are no special variants (like the > rave-sp driver) I would prefer using the module name. We can of course > use more names, like the part number of the the device, but it is very > convenient to also use the module name, and other subsystems also do > that. > > If you want to add specific names for this device please let me know > and I will add them to the list. You know much more about this module > than myself. if we want to use the driver name, then why not build this into the new_device handling itself instead of duplicating that name in each serdev_device_id table. What is the reference here? For example platform device do matching on driver name if I recall this correctly. However what is most important is that device_get_match_data() actually works. There should be no difference between DT, ACPI or a dynamic device. Regards Marcel
Hi Marcel, On Mon, Jun 11, 2018 at 5:52 PM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Ricardo, > > >>>> the commit message is misleading me. If I build something with ACPI or DT support, then modinfo will show all modalias information for ACPI and DT compatible strings. What else does udev/modprobe actually need? Is something broken with the modalias export? > >>> > >>> The main purpose is to autoload drivers for devices that have been > >>> created via sysfs or another module. > >>> > >>> Eg1: We have a serial port on a standard computer that has connected a > >>> GPS module. Since it is something that is not in the ACPI nor the DT > >>> table the user will run > >>> > >>> echo serdev_gps > /sys/bus/serial/devices/serial0/new_device > >>> > >>> Eg2 module: https://github.com/ribalda/linux/blob/415bb3f0076c2b846ebe5409589b8e1e3004f55a/drivers/tty/serdev/test_platform.c > >>> > >>> Modprobe does not know what module to load for that device unless > >>> there is a matching MODULE_DEVICE_TABLE > >>> Today, we have the same functionality for i2c devices > >>> https://www.kernel.org/doc/Documentation/i2c/instantiating-devices > >> > >> but why does this have to be the driver name? I would rather say, create a generic string that describes the hardware and then use that. I think you also want the special handling, as from USB for example, that lets you reference an existing .compatible or ACPI ID as reference so that the driver_data is copied. > > > > We can choose any name, but if there are no special variants (like the > > rave-sp driver) I would prefer using the module name. We can of course > > use more names, like the part number of the the device, but it is very > > convenient to also use the module name, and other subsystems also do > > that. > > > > If you want to add specific names for this device please let me know > > and I will add them to the list. You know much more about this module > > than myself. > > if we want to use the driver name, then why not build this into the new_device handling itself instead of duplicating that name in each serdev_device_id table. What is the reference here? For example platform device do matching on driver name if I recall this correctly. We do the matching also over driver name at serdev_device_match(): if (sdrv->id_table) return !!serdev_match_id(sdrv->id_table, sdev); return strcmp(sdev->modalias, drv->name) == 0; This is just for the module autoloading > > However what is most important is that device_get_match_data() actually works. There should be no difference between DT, ACPI or a dynamic device. Agree, will take a look to this tomorrow morning. > > Regards > > Marcel >
diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c index db6068cd7a1f..6d2ac6cae63f 100644 --- a/drivers/net/ethernet/qualcomm/qca_uart.c +++ b/drivers/net/ethernet/qualcomm/qca_uart.c @@ -405,6 +405,12 @@ static void qca_uart_remove(struct serdev_device *serdev) free_netdev(qca->net_dev); } +static struct serdev_device_id qca_uart_serdev_id[] = { + { QCAUART_DRV_NAME, }, + {} +}; +MODULE_DEVICE_TABLE(serdev, qca_uart_serdev_id); + static struct serdev_device_driver qca_uart_driver = { .probe = qca_uart_probe, .remove = qca_uart_remove, @@ -412,6 +418,7 @@ static struct serdev_device_driver qca_uart_driver = { .name = QCAUART_DRV_NAME, .of_match_table = of_match_ptr(qca_uart_of_match), }, + .id_table = qca_uart_serdev_id, }; module_serdev_device_driver(qca_uart_driver);
Export serdev table to the module header, allowing module autoload via udev/modprobe. Cc: Lino Sanfilippo <LinoSanfilippo@gmx.de> Cc: David S. Miller <davem@davemloft.net> Cc: Stefan Wahren <stefan.wahren@i2se.com> Cc: Rob Herring <robh@kernel.org> Cc: Johan Hovold <johan@kernel.org> Cc: netdev@vger.kernel.org Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/net/ethernet/qualcomm/qca_uart.c | 7 +++++++ 1 file changed, 7 insertions(+)