| Message ID | 1608519279-13341-1-git-send-email-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| Headers | show |
| Series | treewide: bd9571mwv: Add support for BD9574MWF | expand |
On Mon, Dec 21, 2020 at 3:57 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Use dev_regmap_add_irq_chip() to simplify the code. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Shimoda-san, On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > From: Khiem Nguyen <khiem.nguyen.xt@renesas.com> > > The new PMIC BD9574MWF inherits features from BD9571MWV. > Add the support of new PMIC to existing bd9571mwv driver. > > Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@renesas.com> > Co-developed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/mfd/bd9571mwv.c > +++ b/drivers/mfd/bd9571mwv.c > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client, > > static const struct of_device_id bd9571mwv_of_match_table[] = { > { .compatible = "rohm,bd9571mwv", }, > + { .compatible = "rohm,bd9574mwf", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table); > > static const struct i2c_device_id bd9571mwv_id_table[] = { > - { "bd9571mwv", 0 }, > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 }, > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 }, Why add the chip types? These are unused, and the driver uses autodetection of the chip variant anyway. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, 2020-12-22 at 09:49 +0100, Geert Uytterhoeven wrote: > On Mon, Dec 21, 2020 at 3:57 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Use dev_regmap_add_irq_chip() to simplify the code. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> I thought I did review this earlier... Br, Matti Vaittinen
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM > On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: <snip> > > --- a/drivers/mfd/bd9571mwv.c > > +++ b/drivers/mfd/bd9571mwv.c > > > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client, > > > > static const struct of_device_id bd9571mwv_of_match_table[] = { > > { .compatible = "rohm,bd9571mwv", }, > > + { .compatible = "rohm,bd9574mwf", }, > > { /* sentinel */ } > > }; > > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table); > > > > static const struct i2c_device_id bd9571mwv_id_table[] = { > > - { "bd9571mwv", 0 }, > > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 }, > > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 }, > > Why add the chip types? These are unused, and the driver uses > autodetection of the chip variant anyway. I just added the chip types in the future use. As you said, these are unused and we should not add a future use in general. So, I'll remove this change. Also, I think I should remove the following patch. [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support Best regards, Yoshihiro Shimoda
Hi Matti-san, > From: Vaittinen, Matti, Sent: Tuesday, December 22, 2020 6:15 PM > > On Tue, 2020-12-22 at 09:49 +0100, Geert Uytterhoeven wrote: > > On Mon, Dec 21, 2020 at 3:57 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Use dev_regmap_add_irq_chip() to simplify the code. > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> Thank you for your review! > I thought I did review this earlier... You're correct. I'm sorry, I completely overlooked your Reviewed-by tag in previous. Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Tue, Dec 22, 2020 at 10:23 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM > > On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > <snip> > > > --- a/drivers/mfd/bd9571mwv.c > > > +++ b/drivers/mfd/bd9571mwv.c > > > > > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct i2c_client *client, > > > > > > static const struct of_device_id bd9571mwv_of_match_table[] = { > > > { .compatible = "rohm,bd9571mwv", }, > > > + { .compatible = "rohm,bd9574mwf", }, > > > { /* sentinel */ } > > > }; > > > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table); > > > > > > static const struct i2c_device_id bd9571mwv_id_table[] = { > > > - { "bd9571mwv", 0 }, > > > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 }, > > > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 }, > > > > Why add the chip types? These are unused, and the driver uses > > autodetection of the chip variant anyway. > > I just added the chip types in the future use. As you said, > these are unused and we should not add a future use in general. > So, I'll remove this change. OK. > Also, I think I should remove the following patch. > > [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support You mean removing the chip types from bd9571mwv_gpio_id_table[]? You still need the "bd9574mwf-gpio" entry, don't you? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, 2020-12-22 at 09:25 +0000, Yoshihiro Shimoda wrote: > Hi Matti-san, > > > From: Vaittinen, Matti, Sent: Tuesday, December 22, 2020 6:15 PM > > > > On Tue, 2020-12-22 at 09:49 +0100, Geert Uytterhoeven wrote: > > > On Mon, Dec 21, 2020 at 3:57 AM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > Use dev_regmap_add_irq_chip() to simplify the code. > > > > > > > > Signed-off-by: Yoshihiro Shimoda < > > > > yoshihiro.shimoda.uh@renesas.com> > > > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > Thank you for your review! > > > I thought I did review this earlier... > > You're correct. I'm sorry, I completely overlooked your Reviewed-by > tag in previous. It happens :) But of you re-spin the series, please revise the tags for patches. I think I did review almost all of them (except the SPDX- patches as I am not competent on licenses...) > > Best regards, > Yoshihiro Shimoda >
On Tue, 2020-12-22 at 09:23 +0000, Yoshihiro Shimoda wrote: > Hi Geert-san, > > Thank you for your review! > > > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 5:53 PM > > On Mon, Dec 21, 2020 at 3:56 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > <snip> > > > --- a/drivers/mfd/bd9571mwv.c > > > +++ b/drivers/mfd/bd9571mwv.c > > > @@ -200,12 +277,14 @@ static int bd9571mwv_probe(struct > > > i2c_client *client, > > > > > > static const struct of_device_id bd9571mwv_of_match_table[] = { > > > { .compatible = "rohm,bd9571mwv", }, > > > + { .compatible = "rohm,bd9574mwf", }, > > > { /* sentinel */ } > > > }; > > > MODULE_DEVICE_TABLE(of, bd9571mwv_of_match_table); > > > > > > static const struct i2c_device_id bd9571mwv_id_table[] = { > > > - { "bd9571mwv", 0 }, > > > + { "bd9571mwv", ROHM_CHIP_TYPE_BD9571 }, > > > + { "bd9574mwf", ROHM_CHIP_TYPE_BD9574 }, > > > > Why add the chip types? These are unused, and the driver uses > > autodetection of the chip variant anyway. > > I just added the chip types in the future use. As you said, > these are unused and we should not add a future use in general. > So, I'll remove this change. > > Also, I think I should remove the following patch. > > [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support I think this depends on whether you wish to add support for the > "RECOV_GPOUT", "FREQSEL" and "RTC_IN", which you mention in GPIO commit message. If you plan on adding the support, then you need to differentiate the ICs on GPIO driver, right? Best Regards Matti Vaittinen
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, December 22, 2020 6:32 PM <snip> > > Also, I think I should remove the following patch. > > > > [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support > > You mean removing the chip types from bd9571mwv_gpio_id_table[]? > You still need the "bd9574mwf-gpio" entry, don't you? You're correct. The MFD driver uses "bd9574mwf-gpio". And, "bd9574mwf-gpio" with 0 is not good. So, I'll keep this patch. Thank you for the point it out. Best regards, Yoshihiro Shimoda
Hi Matti-san, > From: Vaittinen, Matti, Sent: Tuesday, December 22, 2020 6:39 PM <snip> > > Also, I think I should remove the following patch. > > > > [PATCH v4 08/12] gpio: bd9571mwv: Add BD9574MWF support > > I think this depends on whether you wish to add support for the > > > "RECOV_GPOUT", "FREQSEL" and "RTC_IN", > > which you mention in GPIO commit message. If you plan on adding the > support, then you need to differentiate the ICs on GPIO driver, right? Thank you for the reply. As I replied to Geert-san, at least this MFD driver uses "bd9574mwf-gpio" for probing. So, I'll keep that patch as-is. Best regards, Yoshihiro Shimoda
Hi Matti-san, > From: Vaittinen, Matti, Sent: Tuesday, December 22, 2020 6:39 PM > > On Tue, 2020-12-22 at 09:25 +0000, Yoshihiro Shimoda wrote: > > Hi Matti-san, > > > > > From: Vaittinen, Matti, Sent: Tuesday, December 22, 2020 6:15 PM > > > > > > On Tue, 2020-12-22 at 09:49 +0100, Geert Uytterhoeven wrote: > > > > On Mon, Dec 21, 2020 at 3:57 AM Yoshihiro Shimoda > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: <snip> > > > I thought I did review this earlier... > > > > You're correct. I'm sorry, I completely overlooked your Reviewed-by > > tag in previous. > > It happens :) But of you re-spin the series, please revise the tags for > patches. I think I did review almost all of them (except the SPDX- > patches as I am not competent on licenses...) I got it. I'll add your Reviewed-by tags. Best regards, Yoshihiro Shimoda