netdev - Re: [PATCH v9 08/24] wfx: add bus

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

Message-ID: <20220112114332.jadw527pe7r2j4vv@pali>
Date:   Wed, 12 Jan 2022 12:43:32 +0100
From:   Pali Rohár <pali@...nel.org>
To:     Jérôme Pouiller <jerome.pouiller@...abs.com>
Cc:     linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        Kalle Valo <kvalo@...eaurora.org>, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S . Miller" <davem@...emloft.net>,
        devicetree@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        linux-mmc@...r.kernel.org, Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v9 08/24] wfx: add bus_sdio.c

On Wednesday 12 January 2022 12:18:58 Jérôme Pouiller wrote:
> On Wednesday 12 January 2022 11:58:59 CET Pali Rohár wrote:
> > On Tuesday 11 January 2022 18:14:08 Jerome Pouiller wrote:
> > > +static const struct sdio_device_id wfx_sdio_ids[] = {
> > > +     { SDIO_DEVICE(SDIO_VENDOR_ID_SILABS, SDIO_DEVICE_ID_SILABS_WF200) },
> > > +     { },
> > > +};
> > 
> > Hello! Is this table still required?
> 
> As far as I understand, if the driver does not provide an id_table, the
> probe function won't be never called (see sdio_match_device()).
> 
> Since, we rely on the device tree, we could replace SDIO_VENDOR_ID_SILABS
> and SDIO_DEVICE_ID_SILABS_WF200 by SDIO_ANY_ID. However, it does not hurt
> to add an extra filter here.

Now when this particular id is not required, I'm thinking if it is still
required and it is a good idea to define these SDIO_VENDOR_ID_SILABS
macros into kernel include files. As it would mean that other broken
SDIO devices could define these bogus numbers too... And having them in
common kernel includes files can cause issues... e.g. other developers
could think that it is correct to use them as they are defined in common
header files. But as these numbers are not reliable (other broken cards
may have same ids as wf200) and their usage may cause issues in future.

Ulf, any opinion?

Btw, is there any project which maintains SDIO ids, like there is
pci-ids.ucw.cz for PCI or www.linux-usb.org/usb-ids.html for USB?

> > > +MODULE_DEVICE_TABLE(sdio, wfx_sdio_ids);
> > > +
> > > +struct sdio_driver wfx_sdio_driver = {
> > > +     .name = "wfx-sdio",
> > > +     .id_table = wfx_sdio_ids,
> > > +     .probe = wfx_sdio_probe,
> > > +     .remove = wfx_sdio_remove,
> > > +     .drv = {
> > > +             .owner = THIS_MODULE,
> > > +             .of_match_table = wfx_sdio_of_match,
> > > +     }
> > > +};
> > > --
> > > 2.34.1
> > >
> > 
> 
> 
> -- 
> Jérôme Pouiller
> 
> 
>