Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   =?ISO-8859-1?Q?J=E9r=F4me?= Pouiller <jerome.pouiller@silabs.com>
To:     Pali =?ISO-8859-1?Q?Roh=E1r?= <pali@kernel.org>
Cc:     linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        Kalle Valo <kvalo@codeaurora.org>, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "David S . Miller" <davem@davemloft.net>,
        devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
        linux-mmc@vger.kernel.org, Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH v9 08/24] wfx: add bus_sdio.c
Date:   Wed, 12 Jan 2022 19:23:59 +0100
Message-ID: <1655654.vHqhSpDN13@pc-42>
Organization: Silicon Labs
In-Reply-To: <20220112174848.db5osolurllpc7du@pali>
References: <20220111171424.862764-1-Jerome.Pouiller@silabs.com> <2680707.qJCEgCfB62@pc-42> <20220112174848.db5osolurllpc7du@pali>
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?cu9kwBcxxXHYt7506+JZAM2m/ldMQLLq9vlZ2OAhY2yQs/cU43cDGXjC7T?=
 =?iso-8859-1?Q?RGZgiE854SFwvimRXRVK24XQlt7Pd8mMTq7GOV7DgS0Jg2QQd67lGIhMRd?=
 =?iso-8859-1?Q?kUJEtu0xLTXWbkNgt1bE9qkMafz2jnuMsbtjzcSN+HS+pg6ao/5QxPA/EJ?=
 =?iso-8859-1?Q?nxjpQOmtpgDm6lTc3PqHmBDVUzML2rNucsxqSNi0QZYDbvxFIzV1qztj6y?=
 =?iso-8859-1?Q?wlIpTve7qTPQ/QltVqvIaFWuaS2U4vlF8b+gHlhX1thk/SCGxZALBkb+d9?=
 =?iso-8859-1?Q?yA3DKxMPnnmPmDt2VtGAooUHYYyykH8ED2JKq5tqAnMpNW4rPopLArA2h0?=
 =?iso-8859-1?Q?jG0z/WZRQ0TCsBaJjfT3f97Lgv5+0gTeb3QnjWiQrvBLcPllyOj18cKiE1?=
 =?iso-8859-1?Q?tdSSimdGyOBN+wyALdYdpotnNAvFUBrGwCY7uf3xqkUM5AYJcZY/fJ7GBd?=
 =?iso-8859-1?Q?fOImaVaA2bukCcu7UBPQrFO5iwx5kCYje19YkYUGBXH3xYQXPSDAP65zAW?=
 =?iso-8859-1?Q?WL/CHHsxBE6SnOErh0WJkBQbf/4MOnJgTIQalWLGNOkOhvbWazW3iIUddD?=
 =?iso-8859-1?Q?xkkCAwI03Eg63XgPtX9LeZC8Q7O2P0AqyhAlwSxgxo2WAt/PJLYwY4W2mi?=
 =?iso-8859-1?Q?nqT1hEK9KJjvd0mV9AP1/FnbVnYqLTtN6SliBLQCrgvYGuYudhCskxOLsH?=
 =?iso-8859-1?Q?08ld2yLILuBBo11kHk4+Mc47UFoNFg9LYuwKDsq99Tm4y09fiCTYixPLPW?=
 =?iso-8859-1?Q?0J3io4pcKaba7jAs/AFAh1W8/VwVLJMeDikbsu+jrNRFEe/GoiYFO78X6x?=
 =?iso-8859-1?Q?npbaHCgFS5oxoRjV8vADxKsD1LhFUZv6ANUAtSFFfuD9hThS7deDmkxY2l?=
 =?iso-8859-1?Q?Tfw2BxnzbnTQpjEWs9eWWxDDUxZ+n4nRbQlVYRzE20zpIUdmbz5RAnWQKC?=
 =?iso-8859-1?Q?ZBXW7MAFKbQCHACWJG+IJVORCCLMNDXdZA3JH6yoccXWChqhoR18ZPGw0T?=
 =?iso-8859-1?Q?FpDSbCFAq0zr6jHujqUIVcHwRX+y5GOPY0Qu6FpCfVivTXfnExDrhKtyAY?=
 =?iso-8859-1?Q?ZrXQnSZEV23OjhGcaWcVbm0GAUbzzbC5cUvhMsK9fCEV+5akBBhiDgpRan?=
 =?iso-8859-1?Q?Uo0rx9lYTnwo51YrpOAkgaagcAgmiNDtO7q0DXfOfBIGSqgnvc64PGhm/5?=
 =?iso-8859-1?Q?xzS+I5WOfpaZW6/MgIvt86Hm1TR4QNXfqfYkBiBOCPE0KON7qrGtVwnw8/?=
 =?iso-8859-1?Q?lOKW7Lpu2mjqaAMsLOGjEgyMbUL3cqiqEDAt1pazPFB+E3Vt00RQuZtlrR?=
 =?iso-8859-1?Q?qR3YNUuFiqtPfkS2s41IfpbjvVAtEuaiRs3MCiSEe4etxV0tcr/+3scoZX?=
 =?iso-8859-1?Q?EtUjVdtcqkpkw0XdX1HIacS7/desa/g1ic9uiw3KBBb/I3HUF2+NMDcsHH?=
 =?iso-8859-1?Q?4hvskXBE/NkvwLSUGkA/9uVacI8rZ2DVekpzIlDEKVGc/XkURLvTyFEUMV?=
 =?iso-8859-1?Q?2eNus0UZF9XzCr9AKAVaMqe+9njCst1fnuofkc6Tx4XvEung/sot9gb3oT?=
 =?iso-8859-1?Q?IOlygxnflXG5C8RINs6cJLnFrWlJeQ7zEJdMiraOZBbZ9s3mdLBngb/4N6?=
 =?iso-8859-1?Q?WNw8LpqXhycw8MTxugMZZnPJzx9xR6NNqWhlL7bou6jMyH6iI+jzZspwNW?=
 =?iso-8859-1?Q?tA+FZF+sc5haVTSWvuWEwEjNG67oMiQBIsGdRCkb9/hgMtB4R7DHGeHgyE?=
 =?iso-8859-1?Q?H+D7jciqEYAMChXO/sZwYjdTU=3D?=
X-OriginatorOrg: silabs.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 051097c2-d173-4ea5-b6e1-08d9d5f8b7b3
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5657.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jan 2022 18:24:06.3530
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 54dbd822-5231-4b20-944d-6f4abcd541fb
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4vrYE6RV29z+ymdXcUXnISC+ifM5M6Hti14rcFGL9p3eyksrBF6+IRKUHtY2cwgKiEGKX7z6jx0SMhB2L7RQxg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5172
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

On Wednesday 12 January 2022 18:48:48 CET Pali Roh=E1r wrote:
> CAUTION: This email originated from outside of the organization. Do not c=
lick links or open attachments unless you recognize the sender and know the=
 content is safe.
>=20
>=20
> On Wednesday 12 January 2022 17:45:45 J=E9r=F4me Pouiller wrote:
> > On Wednesday 12 January 2022 12:43:32 CET Pali Roh=E1r wrote:
> > >
> > > On Wednesday 12 January 2022 12:18:58 J=E9r=F4me Pouiller wrote:
> > > > On Wednesday 12 January 2022 11:58:59 CET Pali Roh=E1r wrote:
> > > > > On Tuesday 11 January 2022 18:14:08 Jerome Pouiller wrote:
> > > > > > +static const struct sdio_device_id wfx_sdio_ids[] =3D {
> > > > > > +     { SDIO_DEVICE(SDIO_VENDOR_ID_SILABS, SDIO_DEVICE_ID_SILAB=
S_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 no=
t hurt
> > > > to add an extra filter here.
> > >
> > > Now when this particular id is not required, I'm thinking if it is st=
ill
> > > 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 developer=
s
> > > could think that it is correct to use them as they are defined in com=
mon
> > > header files. But as these numbers are not reliable (other broken car=
ds
> > > may have same ids as wf200) and their usage may cause issues in futur=
e.
> >
> > In order to make SDIO_VENDOR_ID_SILABS less official, do you prefer to
> > define it in wfx/bus_sdio.c instead of mmc/sdio_ids.h?
> >
> > Or even not defined at all like:
> >
> >     static const struct sdio_device_id wfx_sdio_ids[] =3D {
> >          /* WF200 does not have official VID/PID */
> >          { SDIO_DEVICE(0x0000, 0x1000) },
> >          { },
> >     };
>=20
> This has advantage that it is explicitly visible that this device does
> not use any officially assigned ids.

Ulf, are you also agree?


--=20
J=E9r=F4me Pouiller