netdev - Re: [PATCH v2 15/24] net: qualcomm: MODULE_DEVICE

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] [day] [month] [year] [list]

Message-ID: <CAPybu_37067-Vny5ZbMK6ZkU8=f-Pk8f9=Sf+NsK7V_VGBMBLg@mail.gmail.com>
Date:   Mon, 11 Jun 2018 18:21:56 +0200
From:   Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
To:     Marcel Holtmann <marcel@...tmann.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        Lino Sanfilippo <LinoSanfilippo@....de>,
        David Miller <davem@...emloft.net>,
        Stefan Wahren <stefan.wahren@...e.com>,
        Rob Herring <robh@...nel.org>, Johan Hovold <johan@...nel.org>,
        netdev@...r.kernel.org
Subject: Re: [PATCH v2 15/24] net: qualcomm: MODULE_DEVICE_TABLE(serdev)

Hi Marcel,
On Mon, Jun 11, 2018 at 5:52 PM Marcel Holtmann <marcel@...tmann.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
>


-- 
Ricardo Ribalda