diff mbox series

[289/606] macintosh: ams/ams-i2c: Convert to i2c's .probe_new()

Message ID 20221118224540.619276-290-uwe@kleine-koenig.org
State Not Applicable
Headers show
Series i2c: Complete conversion to i2c_probe_new | expand

Commit Message

Uwe Kleine-König Nov. 18, 2022, 10:40 p.m. UTC
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

The probe function doesn't make use of the i2c_device_id * parameter so it
can be trivially converted.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/macintosh/ams/ams-i2c.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Christophe Leroy Nov. 19, 2022, 7:38 a.m. UTC | #1
Le 18/11/2022 à 23:40, Uwe Kleine-König a écrit :
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> The probe function doesn't make use of the i2c_device_id * parameter so it
> can be trivially converted.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

The patch itself and the others seems ok. But can you group all 
macintosh changes into a single patch instead of the 9 patches you sent ?

See the process about submitting patches, 
https://docs.kernel.org/process/submitting-patches.html and especially 
the "NO!!!! No more huge patch bombs to linux-kernel@vger.kernel.org 
people!" and the associated reference 
https://lore.kernel.org/all/20050711.125305.08322243.davem@davemloft.net/ :

	If you feel the need to send, say, more than 15 patches at once, 
reconsider.

Thanks
Christophe

> ---
>   drivers/macintosh/ams/ams-i2c.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/macintosh/ams/ams-i2c.c b/drivers/macintosh/ams/ams-i2c.c
> index 3ded340699fb..a4a1035eb412 100644
> --- a/drivers/macintosh/ams/ams-i2c.c
> +++ b/drivers/macintosh/ams/ams-i2c.c
> @@ -56,8 +56,7 @@ enum ams_i2c_cmd {
>   	AMS_CMD_START,
>   };
>   
> -static int ams_i2c_probe(struct i2c_client *client,
> -			 const struct i2c_device_id *id);
> +static int ams_i2c_probe(struct i2c_client *client);
>   static void ams_i2c_remove(struct i2c_client *client);
>   
>   static const struct i2c_device_id ams_id[] = {
> @@ -70,7 +69,7 @@ static struct i2c_driver ams_i2c_driver = {
>   	.driver = {
>   		.name   = "ams",
>   	},
> -	.probe          = ams_i2c_probe,
> +	.probe_new      = ams_i2c_probe,
>   	.remove         = ams_i2c_remove,
>   	.id_table       = ams_id,
>   };
> @@ -155,8 +154,7 @@ static void ams_i2c_get_xyz(s8 *x, s8 *y, s8 *z)
>   	*z = ams_i2c_read(AMS_DATAZ);
>   }
>   
> -static int ams_i2c_probe(struct i2c_client *client,
> -			 const struct i2c_device_id *id)
> +static int ams_i2c_probe(struct i2c_client *client)
>   {
>   	int vmaj, vmin;
>   	int result;
Uwe Kleine-König Nov. 19, 2022, 10:16 a.m. UTC | #2
Hello Christophe,

On Sat, Nov 19, 2022 at 07:38:58AM +0000, Christophe Leroy wrote:
> Le 18/11/2022 à 23:40, Uwe Kleine-König a écrit :
> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > The probe function doesn't make use of the i2c_device_id * parameter so it
> > can be trivially converted.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> The patch itself and the others seems ok. But can you group all 
> macintosh changes into a single patch instead of the 9 patches you sent ?
> 
> See the process about submitting patches, 
> https://docs.kernel.org/process/submitting-patches.html and especially 
> the "NO!!!! No more huge patch bombs to linux-kernel@vger.kernel.org 
> people!" and the associated reference 
> https://lore.kernel.org/all/20050711.125305.08322243.davem@davemloft.net/ :
> 
> 	If you feel the need to send, say, more than 15 patches at once, 
> reconsider.

Thanks for your feedback.

Let me point out in response to the request to squash patches together
that this wish doesn't seem to be the universal right thing to do.

Last time I did a conversion grouped by subsystem, one of the replies
was:

	Reviewed-by: ...
	if you split on per driver basis.

(https://lore.kernel.org/linux-iio/20220808085526.280066-1-u.kleine-koenig@pengutronix.de)

The obvious upside of a split per driver is that the individual bits are
easier to review, an Ack can be limited to a certain set of drivers, and
(probably most important): If a driver later gets a commit that needs
backporting, a patch that only touches this single driver can more
easily be backported as a dependency.

Also the request to not send everything at once but keep back some
patches has an obvious downside. I.e. I have to restart yet more often
to rebase + test to get all patches applied in the end. And you could
argue that the sum of the work load on the mail servers doesn't get
smaller in sum if the 600 patches are sent over (say) 2 months.
(To prevent a flame war here: Yeah, I agree there are downsides of a
single big series, too.)

All in all my personal opinion is that the advantages of a split per
driver slightly outweight the disadvantages. But obviously that's
subjective.

I will wait a while before sending updates to this series, so don't
expect a new patch very soon. (This has the nice effect that it gives
you some time to reconsider your argument and maybe apply the patches as
they are now, preventing another mail to the lists with the cumulated
macintosh changes ;-)

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/macintosh/ams/ams-i2c.c b/drivers/macintosh/ams/ams-i2c.c
index 3ded340699fb..a4a1035eb412 100644
--- a/drivers/macintosh/ams/ams-i2c.c
+++ b/drivers/macintosh/ams/ams-i2c.c
@@ -56,8 +56,7 @@  enum ams_i2c_cmd {
 	AMS_CMD_START,
 };
 
-static int ams_i2c_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id);
+static int ams_i2c_probe(struct i2c_client *client);
 static void ams_i2c_remove(struct i2c_client *client);
 
 static const struct i2c_device_id ams_id[] = {
@@ -70,7 +69,7 @@  static struct i2c_driver ams_i2c_driver = {
 	.driver = {
 		.name   = "ams",
 	},
-	.probe          = ams_i2c_probe,
+	.probe_new      = ams_i2c_probe,
 	.remove         = ams_i2c_remove,
 	.id_table       = ams_id,
 };
@@ -155,8 +154,7 @@  static void ams_i2c_get_xyz(s8 *x, s8 *y, s8 *z)
 	*z = ams_i2c_read(AMS_DATAZ);
 }
 
-static int ams_i2c_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int ams_i2c_probe(struct i2c_client *client)
 {
 	int vmaj, vmin;
 	int result;