Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Biju Das <biju.das.jz@bp.renesas.com>
To:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC:     Mark Brown <broonie@kernel.org>,
        Mike Looijmans <mike.looijmans@topic.nl>,
        Andreas Helbech Kleist <andreaskleist@gmail.com>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        =?iso-8859-1?Q?Uwe_Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
Thread-Topic: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
Thread-Index: AQHZuLEfke7PxT5AM0ulEoLw/IRIvq++HaYAgAAHzdCAAB6ZAIAAA3QAgAAChWCAAZImAIAAxbxwgAQs5wCAAUhbgIAAfjkAgABL7XA=
Date:   Sun, 23 Jul 2023 06:05:06 +0000
Message-ID: <OS0PR01MB59222E6CE0EEED3A5FE843C1863DA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
References: <20230717131756.240645-1-biju.das.jz@bp.renesas.com>
 <20230717131756.240645-2-biju.das.jz@bp.renesas.com>
 <ZLVlIPHdmEGdG5D1@google.com>
 <OS0PR01MB592264ADC86061AB003048DD863BA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
 <66228ad3-c087-418c-925f-b9fd4a60fb16@sirena.org.uk>
 <ZLWIPPAfeR1+KbeH@google.com>
 <OS0PR01MB59225D8CF3E96808DD776A8E863BA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
 <ZLbbslBiIQXFWpmN@google.com>
 <TYCPR01MB593346FBBA320260A290EAFD8639A@TYCPR01MB5933.jpnprd01.prod.outlook.com>
 <ZLsCOj1t4JvG3SEp@google.com>
 <TYCPR01MB5933D4252360AAE57D90FE1C863CA@TYCPR01MB5933.jpnprd01.prod.outlook.com>
 <CAKdAkRT9tMnLnDLgWAevE_4HQ0wYMPehvsYaAeYrXdGGiyjXRA@mail.gmail.com>
In-Reply-To: <CAKdAkRT9tMnLnDLgWAevE_4HQ0wYMPehvsYaAeYrXdGGiyjXRA@mail.gmail.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?HjOLTCgL8IftxM6hoopdCwkHwX/VL8cs0sK69rNWeVWdSWnUswk3UzAjjH?=
 =?iso-8859-1?Q?y6SMMICjWN1NikGn9eUnwIIOWQkgZCA5tC7LNOCAAwTxn6/vRyLb7zZVsT?=
 =?iso-8859-1?Q?SwXX5oXAEow2f56JbPVouq3DflDjBnFFcFnUAmvnq+rr7fvZSbj7+Pmxdg?=
 =?iso-8859-1?Q?kyJpSwTDJ9HBnpOIzP1gPsDB4T7Sls3Q0k8vdrENEaEpO6qdKt/GHMpPNm?=
 =?iso-8859-1?Q?NeHuF1mpjJn7ivdLVFvcbEvQLxedlgd8Mq7HrVw2yW0d6yc9usoUSM7tRu?=
 =?iso-8859-1?Q?wEkz3sepUHOucG7rjz955xLbpr6wbXJtp0nhKiTn86UlPKbNV8ufRXFyA2?=
 =?iso-8859-1?Q?/d2R0bmTPLtX8vWxqiwXg6s0wTd1Ekw5IFdQLpGUbqhyN4NwNwy5rFHQI2?=
 =?iso-8859-1?Q?gUTIwt2wF/jHc0BUC6FyJdLlQkQltwmN20xfceFEmkaEi74weqh/Meyzd2?=
 =?iso-8859-1?Q?8dqzsk+HNK2k6A+ML4R+Y5wp0V+x3bnyRH4o0BrsHuebIzHUR4gCF+ccYO?=
 =?iso-8859-1?Q?iGHgWCVLDi2XtNz7IfFbBaMOkt4TcBRLSGLx8WZ77WGt5bzCtLe15G5wyi?=
 =?iso-8859-1?Q?7VQBYsyD5lCI0BUT1nArMnygvGvONbqKnj6aSlgSipn+3OdBAYF/KccBgE?=
 =?iso-8859-1?Q?i7L/Iy6fNPGxYZIwodzYrMwAhuAX8AVsaJRWcGwjoQy9Seq/CGdZRBdD6U?=
 =?iso-8859-1?Q?g+Fpt3f0GaqQkKDVIn0n4LLccgwFt81moz2CYqMycMvQ7GVcHWF5VlOXzH?=
 =?iso-8859-1?Q?TU+jTReBjlKAhoSnbAnyQm74ejuqx8WiAkA4kU0m9NcAg81x9MQGXCLeyb?=
 =?iso-8859-1?Q?cEIuzisoFBHbwRQ8HQ51Afs2mt83VqDvcMvffzfWVr1cQvWP+IETV4kJYe?=
 =?iso-8859-1?Q?2tI+WVLGnLCXTH6W2yTBS3RhwMeCBhsg2UoOYRzHBxB6j0kYJQXqIEeQwT?=
 =?iso-8859-1?Q?blmsXbXnvMoEXqreuJ9xft8wQiLz+r6buU+pM0H9TNxlCB/ESW9+f5htfk?=
 =?iso-8859-1?Q?z/hIYVJjOMtW/5SZemJrPvhiDp3FQR6j7t0bARkCSK/q7p/j8YhakRHoc6?=
 =?iso-8859-1?Q?xMVVpT2Of8yOqccHYMF4OXELWysu1SrQcEWMYYrMLpbO6ZNL0aPNkSJ5FX?=
 =?iso-8859-1?Q?RNtw49WE8U5RRDQRag82FeSjhdsCWQ5oqfwxjWoODx5p5KqmyBlRei8n06?=
 =?iso-8859-1?Q?QvUGncGTf4wtEED5S5X7bPCWmO/p76ISxsQTdzSHif4LMk23XQkbnZnxyo?=
 =?iso-8859-1?Q?6jbGihrutsfimlefIuuVxBknAR42JI4YMGG/vyVPS7C6GwgJz55SQ8d0Zn?=
 =?iso-8859-1?Q?Y0Y75Ru7E9rucNHVLjatxCWZZsG8w1R4GzDChoSZx4Ud8ueVyFoJupw6vR?=
 =?iso-8859-1?Q?JxPB0cMmHyGALMcuwI6wajkk25CN1eWAD5FT1yR+hYFRZJREww9482fvCA?=
 =?iso-8859-1?Q?hK87+NHdno+H8RzCtGJOl5TQCyMazPb57yHyBSBq3nfm9N7re6AyrAu8zN?=
 =?iso-8859-1?Q?GHPVOAYtbiQZjPW83kxD+ZlxJSlmTetn/jMQL5rIfWQh/FdhhrceFs6hvj?=
 =?iso-8859-1?Q?whn3bio54B4gNmjOKDFnbUF4I2JPbIK/oE8OzLkdhM+m4xZokOHhR4DPjb?=
 =?iso-8859-1?Q?qmHUoR7JxAYU7IwvE9oSJL/vVRs+JPsUrO?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: bp.renesas.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: OS0PR01MB5922.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: be3e7d15-ed74-463d-84bb-08db8b42c35d
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Jul 2023 06:05:06.4826
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 92QFLinrBOyP22wEfLAAhDYyQAYmntbVe1eNCzhf8rzqvuy2e1Wb/5K7+rJgA6jdF+W8gHbLfAfNQFJPNumETHtWk9jSvBeZ7dobgpuC9kw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYCPR01MB11973
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Dmitry,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
>=20
> On Sat, Jul 22, 2023 at 05:51:17PM +0000, Biju Das wrote:
> > Hi Dmitry Torokhov,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > >
> > > On Wed, Jul 19, 2023 at 06:43:47AM +0000, Biju Das wrote:
> > > > Hi Dmitry Torokhov,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > > >
> > > > > On Mon, Jul 17, 2023 at 06:45:27PM +0000, Biju Das wrote:
> > > > > > Hi Dmitry,
> > > > > >
> > > > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify
> > > > > > > probe()
> > > > > > >
> > > > > > > On Mon, Jul 17, 2023 at 07:15:50PM +0100, Mark Brown wrote:
> > > > > > > > On Mon, Jul 17, 2023 at 04:35:02PM +0000, Biju Das wrote:
> > > > > > > >
> > > > > > > > > The .device_get_match_data callbacks are missing for I2C
> > > > > > > > > and SPI bus
> > > > > > > subsystems.
> > > > > > > > > Can you please throw some lights on this?
> > > > > > > >
> > > > > > > > It's the first time I've ever heard of that callback, I
> > > > > > > > don't know why whoever added it wouldn't have done those
> > > > > > > > buses in particular or if it just didn't happen.  Try
> > > > > > > > adding it and if it works send
> > > > > the patches?
> > > > > > >
> > > > > > > I think there is a disconnect. Right now
> > > > > > > device_get_match_data callbacks are part of
> > > > > > > fwnode_operations. I was proposing to add another optional
> > > > > > > device_get_match_data callback to 'struct
> > > bus_type'
> > > > > > > to allow individual buses control how match data is handled,
> > > > > > > before (or after) jumping into the fwnode-backed
> > > > > > > device_get_match_data
> > > > > callbacks.
> > > > > >
> > > > > > That is what implemented here [1] and [2] right?
> > > > > >
> > > > > >
> > > > > > First it check for fwnode-backed device_get_match_data
> > > > > > callbacks and Fallback is bus-type based match.
> > > > > >
> > > > > > Looks like you are proposing to unify [1] and [2] and you want
> > > > > > the logic to be other way around. ie, first bus-type match,
> > > > > > then fwnode-backed callbacks?
> > > > > >
> > > > >
> > > > > I do not have a strong preference for the ordering, i.e. I think
> > > > > it is perfectly fine to do the generic fwnode-based lookup and
> > > > > if there is no match have bus method called as a fallback,
> > > >
> > > > That involves a bit of work.
> > > >
> > > > const void *device_get_match_data(const struct device *dev);
> > > >
> > > > const struct i2c_device_id *i2c_match_id(const struct
> > > > i2c_device_id
> > > *id,
> > > >                                    const struct i2c_client
> > > > *client);
> > > >
> > > > const struct spi_device_id *spi_get_device_id(const struct
> > > > spi_device *sdev);
> > > >
> > > > Basically, the bus-client driver(such as exc3000) needs to pass
> > > > struct device and device_get_match_data after generic fwnode-based
> > > > lookup, needs to find the bus type based on struct device and call
> > > > a new generic
> > > > void* bus_get_match_data(void*) callback, so that each bus
> > > > interface can do a match.
> > >
> > > Yes, something like this (which does not seem that involved to
> me...):
> >
> > Looks it will work.
> >
> > But there is some 2 additional checks in core code, every driver which
> is not bus type need to go through this checks.
> >
> > Also in Bus specific callback, there are 2 additional checks.
> >
> > So, performance wise [1] is better.
>=20
> I do not believe this is a concern whatsoever: majority of
> architectures/boards have been converted to ACPI/DT, which are being
> matched first as they are now, so the fallback to bus-specific matching
> against bus-specific device ID tables will be very infrequent.
> Additionally, device_get_match_data() is predominantly called from
> driver probe paths, so we need not be concerned with it being used with
> class devices or other kinds of devices not associated with a bus.

Looks like most of the i2c client driver uses similar handling for=20
ACPI/DT and ID tables. If that is the case, it is good to have this
proposed change which will simplify most of the drivers listed in [1]

[1] https://elixir.bootlin.com/linux/latest/A/ident/i2c_match_id

Eg: drivers/hwmon/pmbus/ibm-cffps.c

	enum versions vs =3D cffps_unknown;
	const void *md =3D of_device_get_match_data(&client->dev);
	const struct i2c_device_id *id;

	if (md) {
		vs =3D (enum versions)md;
	} else {
		id =3D i2c_match_id(ibm_cffps_id, client);
		if (id)
			vs =3D (enum versions)id->driver_data;
	}

The above code can be converted to=20
     vs =3D (enum versions)device_get_match_data(&client->dev);

>=20
> >
> > Moreover, we need to avoid code duplication with [1]
> >
> > [1]
>=20
> If and when my proposed solution gets into the kernel we can drop
> i2c_get_match_data() altogether.

Agreed. Will wait for other people's view on this topic.

Cheers,
Biju