| Message ID | 20230505172530.357455-5-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
Hi All, > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > Currently, it is not possible to instantiate the i2c client driver using MFD > cell as it is not a platform driver. Add support for Renesas PMIC > RAA215300 RTC platform driver, so that it can be instantiated by MFD API. > The rtc device is created by using i2c_new_ancillary_device() and it > register as the rtc device by the helper function provided by > rtc-isl2108 driver. Not sure this platform driver has to be placed in RTC subsystem rather than MFD subsystem as PMIC MFD driver, can instantiate it using MFD cell?? The advantage is the header file[1](isl1208.h) will be local to RTC subsystem compared to across subsystem (include/linux/rtc/isl1208.h) and thereby avoids subsystem dependencies. [1] https://lore.kernel.org/linux-renesas-soc/20230505172530.357455-4-biju.das.jz@bp.renesas.com/T/#u Cheers, Biju > > The PMIC driver enables RTC device and share the PMIC revision to RTC > platform driver. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v2: > * New patch > --- > drivers/mfd/Kconfig | 7 +++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/raa215300-rtc.c | 52 +++++++++++++++++++++++++++++++++++++ > drivers/mfd/raa215300.c | 38 ++++++++++++++++++++++++--- > 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 > drivers/mfd/raa215300-rtc.c > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index > 9071b0f27b62..cec20c1f143d 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -318,6 +318,13 @@ config PMIC_RAA215300 > help > Support for the Renesas RAA215300 PMIC. > > +config PMIC_RAA215300_RTC > + tristate "Renesas RAA215300 RTC driver" > + depends on PMIC_RAA215300 > + select RTC_DRV_ISL1208 > + help > + Select this to get support for the Renesas RAA215300 RTC > + > config PMIC_DA903X > bool "Dialog Semiconductor DA9030/DA9034 PMIC Support" > depends on I2C=y > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index > d9c601120bfd..ed4b760e564e 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -127,6 +127,7 @@ obj-$(CONFIG_MCP_UCB1200) += ucb1x00-assabet.o > endif > > obj-$(CONFIG_PMIC_RAA215300) += raa215300.o > +obj-$(CONFIG_PMIC_RAA215300_RTC) += raa215300-rtc.o > > obj-$(CONFIG_PMIC_DA903X) += da903x.o > > diff --git a/drivers/mfd/raa215300-rtc.c b/drivers/mfd/raa215300-rtc.c new > file mode 100644 index 000000000000..309ed34d6cd7 > --- /dev/null > +++ b/drivers/mfd/raa215300-rtc.c > @@ -0,0 +1,52 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Renesas RAA215300 RTC Driver > + * > + * Copyright (C) 2023 Renesas Electronics Corporation */ > + > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/rtc/isl1208.h> > + > +#define RAA215300_RTC_DEFAULT_ADDR 0x6f > + > +static void raa215300_rtc_unregister_device(void *data) { > + i2c_unregister_device(data); > +} > + > +static int raa215300_rtc_probe(struct platform_device *pdev) { > + unsigned int *pmic_version = dev_get_drvdata(pdev->dev.parent); > + struct i2c_client *client; > + int ret; > + > + client = i2c_new_ancillary_device(to_i2c_client(pdev->dev.parent), > + "rtc", RAA215300_RTC_DEFAULT_ADDR); > + if (IS_ERR(client)) > + return PTR_ERR(client); > + > + ret = devm_add_action_or_reset(&pdev->dev, > + raa215300_rtc_unregister_device, client); > + if (ret < 0) > + return ret; > + > + return raa215300_rtc_probe_helper(client, *pmic_version); } > + > +static struct platform_driver raa215300_rtc_driver = { > + .probe = raa215300_rtc_probe, > + .driver = { > + .name = "raa215300-rtc", > + }, > +}; > +module_platform_driver(raa215300_rtc_driver); > + > +MODULE_DESCRIPTION("Renesas RAA215300 MFD RTC Driver"); > +MODULE_ALIAS("platform:raa215300-rtc"); > +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); > +MODULE_LICENSE("GPL"); > +MODULE_SOFTDEP("post: rtc_isl1208"); > diff --git a/drivers/mfd/raa215300.c b/drivers/mfd/raa215300.c index > 5cdd3213e99c..c8bdf96b203f 100644 > --- a/drivers/mfd/raa215300.c > +++ b/drivers/mfd/raa215300.c > @@ -8,6 +8,7 @@ > #include <linux/err.h> > #include <linux/i2c.h> > #include <linux/module.h> > +#include <linux/mfd/core.h> > #include <linux/of.h> > #include <linux/regmap.h> > > @@ -29,13 +30,26 @@ static const struct regmap_config > raa215300_regmap_config = { > .cache_type = REGCACHE_FLAT, > }; > > +static const struct mfd_cell raa215300_rtc_dev[] = { > + { .name = "raa215300-rtc" } > +}; > + > +static void raa215300_remove_devices(void *data) { > + mfd_remove_devices(data); > +} > + > static int raa215300_i2c_probe(struct i2c_client *client) { > struct device *dev = &client->dev; > - unsigned int pmic_version; > + unsigned int *pmic_version; > struct regmap *regmap; > int ret; > > + pmic_version = devm_kzalloc(dev, sizeof(*pmic_version), GFP_KERNEL); > + if (!pmic_version) > + return -ENOMEM; > + > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) > return -EOPNOTSUPP; > > @@ -44,11 +58,29 @@ static int raa215300_i2c_probe(struct i2c_client > *client) > return dev_err_probe(dev, PTR_ERR(regmap), > "regmap i2c init failed\n"); > > - ret = regmap_read(regmap, RAA215300_HW_REV, &pmic_version); > + ret = regmap_read(regmap, RAA215300_HW_REV, pmic_version); > if (ret < 0) > return dev_err_probe(dev, ret, "HW rev read failed\n"); > > - dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", pmic_version); > + dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", *pmic_version); > + dev_set_drvdata(dev, pmic_version); > + > + if (of_property_read_bool(dev->of_node, "renesas,rtc-enabled")) { > + /* Enable RTC block */ > + regmap_update_bits(regmap, RAA215300_REG_BLOCK_EN, > + RAA215300_REG_BLOCK_EN_RTC_EN, > + RAA215300_REG_BLOCK_EN_RTC_EN); > + > + ret = mfd_add_devices(dev, 0, raa215300_rtc_dev, > + ARRAY_SIZE(raa215300_rtc_dev), NULL, > + 0, NULL); > + if (ret < 0) > + return ret; > + > + ret = devm_add_action_or_reset(dev, raa215300_remove_devices, > dev); > + if (ret < 0) > + return ret; > + } > > return 0; > } > -- > 2.25.1
Hi Biju, On Tue, May 9, 2023 at 9:06 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > > > Currently, it is not possible to instantiate the i2c client driver using MFD > > cell as it is not a platform driver. Add support for Renesas PMIC > > RAA215300 RTC platform driver, so that it can be instantiated by MFD API. > > The rtc device is created by using i2c_new_ancillary_device() and it > > register as the rtc device by the helper function provided by > > rtc-isl2108 driver. > > Not sure this platform driver has to be placed in RTC subsystem rather than MFD subsystem > as PMIC MFD driver, can instantiate it using MFD cell?? Can't you just instantiate the i2c ancillary device from the PMIC driver, and drop the new mfd and platform rtc drivers? Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > Hi Biju, > > On Tue, May 9, 2023 at 9:06 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > > > > > Currently, it is not possible to instantiate the i2c client driver > > > using MFD cell as it is not a platform driver. Add support for > > > Renesas PMIC > > > RAA215300 RTC platform driver, so that it can be instantiated by MFD > API. > > > The rtc device is created by using i2c_new_ancillary_device() and it > > > register as the rtc device by the helper function provided by > > > rtc-isl2108 driver. > > > > Not sure this platform driver has to be placed in RTC subsystem rather > > than MFD subsystem as PMIC MFD driver, can instantiate it using MFD cell?? > > Can't you just instantiate the i2c ancillary device from the PMIC driver, > and drop the new mfd and platform rtc drivers? Yes, it is possible. The only reason MFD is used for future expansion like adding support for 1) battery charger Or 2) regulator In that case, we can share regmap to sub devices. But these drivers are not currently planned. Apart from that, 1) It avoids subsystem dependencies, ie, PMIC driver directly calling rtc driver for registering RTC device. 2) I guess, the current split will also give some modular design. We have a control to enable or disable the driver. ie, Enable the driver PMIC with RTC enabled and disable the driver PMIC with RTC not enabled(ie, XIN/XOUT grounded) in the system design by controlling the config "PMIC_RAA215300_RTC". These are the reasons for creating MFD driver and platform device. I may be totally wrong. Please correct me if that is the case. Cheers, Biju
Hi Biju, On Tue, May 9, 2023 at 9:35 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > On Tue, May 9, 2023 at 9:06 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > > > > > > > Currently, it is not possible to instantiate the i2c client driver > > > > using MFD cell as it is not a platform driver. Add support for > > > > Renesas PMIC > > > > RAA215300 RTC platform driver, so that it can be instantiated by MFD > > API. > > > > The rtc device is created by using i2c_new_ancillary_device() and it > > > > register as the rtc device by the helper function provided by > > > > rtc-isl2108 driver. > > > > > > Not sure this platform driver has to be placed in RTC subsystem rather > > > than MFD subsystem as PMIC MFD driver, can instantiate it using MFD cell?? > > > > Can't you just instantiate the i2c ancillary device from the PMIC driver, > > and drop the new mfd and platform rtc drivers? > > Yes, it is possible. > > The only reason MFD is used for future expansion like adding support for > > 1) battery charger > Or > 2) regulator I'd just start with a skeleton regulator ("PMIC") driver... > In that case, we can share regmap to sub devices. But these drivers are not currently planned. > > Apart from that, > > 1) It avoids subsystem dependencies, ie, PMIC driver directly calling rtc driver > for registering RTC device. You mean the direct call into raa215300_rtc_probe_helper()? I think that can be solved by enhancing i2c_new_ancillary_device() to take a proper device name, instead of using "dummy"? > 2) I guess, the current split will also give some modular design. > We have a control to enable or disable the driver. > ie, Enable the driver PMIC with RTC enabled and > disable the driver PMIC with RTC not enabled(ie, XIN/XOUT grounded) in the system design > by controlling the config "PMIC_RAA215300_RTC". You already have CONFIG_RTC_DRV_ISL1208, and look at "renesas,rtc-enabled" before activating the RTC? > These are the reasons for creating MFD driver and platform device. > > I may be totally wrong. Please correct me if that is the case. Gr{oetje,eeting}s, Geert
CC Wolfram On Tue, May 9, 2023 at 11:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Tue, May 9, 2023 at 9:35 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > > On Tue, May 9, 2023 at 9:06 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > > > > > > > > > Currently, it is not possible to instantiate the i2c client driver > > > > > using MFD cell as it is not a platform driver. Add support for > > > > > Renesas PMIC > > > > > RAA215300 RTC platform driver, so that it can be instantiated by MFD > > > API. > > > > > The rtc device is created by using i2c_new_ancillary_device() and it > > > > > register as the rtc device by the helper function provided by > > > > > rtc-isl2108 driver. > > > > > > > > Not sure this platform driver has to be placed in RTC subsystem rather > > > > than MFD subsystem as PMIC MFD driver, can instantiate it using MFD cell?? > > > > > > Can't you just instantiate the i2c ancillary device from the PMIC driver, > > > and drop the new mfd and platform rtc drivers? > > > > Yes, it is possible. > > > > The only reason MFD is used for future expansion like adding support for > > > > 1) battery charger > > Or > > 2) regulator > > I'd just start with a skeleton regulator ("PMIC") driver... > > > In that case, we can share regmap to sub devices. But these drivers are not currently planned. > > > > Apart from that, > > > > 1) It avoids subsystem dependencies, ie, PMIC driver directly calling rtc driver > > for registering RTC device. > > You mean the direct call into raa215300_rtc_probe_helper()? > I think that can be solved by enhancing i2c_new_ancillary_device() to > take a proper device name, instead of using "dummy"? > > > 2) I guess, the current split will also give some modular design. > > We have a control to enable or disable the driver. > > ie, Enable the driver PMIC with RTC enabled and > > disable the driver PMIC with RTC not enabled(ie, XIN/XOUT grounded) in the system design > > by controlling the config "PMIC_RAA215300_RTC". > > You already have CONFIG_RTC_DRV_ISL1208, and look at > "renesas,rtc-enabled" before activating the RTC? > > > These are the reasons for creating MFD driver and platform device. > > > > I may be totally wrong. Please correct me if that is the case. Gr{oetje,eeting}s, Geert
Hi Geert, > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > Hi Biju, > > On Tue, May 9, 2023 at 9:35 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > driver On Tue, May 9, 2023 at 9:06 AM Biju Das > <biju.das.jz@bp.renesas.com> wrote: > > > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > > > driver > > > > > > > > > > Currently, it is not possible to instantiate the i2c client > > > > > driver using MFD cell as it is not a platform driver. Add > > > > > support for Renesas PMIC > > > > > RAA215300 RTC platform driver, so that it can be instantiated by > > > > > MFD > > > API. > > > > > The rtc device is created by using i2c_new_ancillary_device() > > > > > and it register as the rtc device by the helper function > > > > > provided by > > > > > rtc-isl2108 driver. > > > > > > > > Not sure this platform driver has to be placed in RTC subsystem > > > > rather than MFD subsystem as PMIC MFD driver, can instantiate it using > MFD cell?? > > > > > > Can't you just instantiate the i2c ancillary device from the PMIC > > > driver, and drop the new mfd and platform rtc drivers? > > > > Yes, it is possible. > > > > The only reason MFD is used for future expansion like adding support > > for > > > > 1) battery charger > > Or > > 2) regulator > > I'd just start with a skeleton regulator ("PMIC") driver... > > > In that case, we can share regmap to sub devices. But these drivers are > not currently planned. > > > > Apart from that, > > > > 1) It avoids subsystem dependencies, ie, PMIC driver directly calling rtc > driver > > for registering RTC device. > > You mean the direct call into raa215300_rtc_probe_helper()? Yes, indeed. > I think that can be solved by enhancing i2c_new_ancillary_device() to take a > proper device name, instead of using "dummy"? Geert, Wolfram, Any pointers for instantiating i2c_client device(for eg: rtc_isl1208.c) by enhancing i2c_new_ancillary_device() api? Apart for this, since a single compatible(the pmic one) is used, In rtc device probe, How do we identify this PMIC RTC device and use the "TYPE_ISL1208" config in RTC driver, once we instantiate RTC device? > > > 2) I guess, the current split will also give some modular design. > > We have a control to enable or disable the driver. > > ie, Enable the driver PMIC with RTC enabled and > > disable the driver PMIC with RTC not enabled(ie, XIN/XOUT grounded) in > the system design > > by controlling the config "PMIC_RAA215300_RTC". > > You already have CONFIG_RTC_DRV_ISL1208, and look at "renesas,rtc-enabled" > before activating the RTC? I agree. this will take care. I just added this CONFIG, so that we can have Built-in PMIC driver and RTC module driver?? Cheers, Biju > > > These are the reasons for creating MFD driver and platform device. > > > > I may be totally wrong. Please correct me if that is the case.
Hi Biju, On Tue, May 9, 2023 at 11:32 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > On Tue, May 9, 2023 at 9:35 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > > driver On Tue, May 9, 2023 at 9:06 AM Biju Das > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > > > > driver > > > > > > > > > > > > Currently, it is not possible to instantiate the i2c client > > > > > > driver using MFD cell as it is not a platform driver. Add > > > > > > support for Renesas PMIC > > > > > > RAA215300 RTC platform driver, so that it can be instantiated by > > > > > > MFD > > > > API. > > > > > > The rtc device is created by using i2c_new_ancillary_device() > > > > > > and it register as the rtc device by the helper function > > > > > > provided by > > > > > > rtc-isl2108 driver. > > > > > > > > > > Not sure this platform driver has to be placed in RTC subsystem > > > > > rather than MFD subsystem as PMIC MFD driver, can instantiate it using > > MFD cell?? > > > > > > > > Can't you just instantiate the i2c ancillary device from the PMIC > > > > driver, and drop the new mfd and platform rtc drivers? > > > > > > Yes, it is possible. > > > > > > The only reason MFD is used for future expansion like adding support > > > for > > > > > > 1) battery charger > > > Or > > > 2) regulator > > > > I'd just start with a skeleton regulator ("PMIC") driver... > > > > > In that case, we can share regmap to sub devices. But these drivers are > > not currently planned. > > > > > > Apart from that, > > > > > > 1) It avoids subsystem dependencies, ie, PMIC driver directly calling rtc > > driver > > > for registering RTC device. > > > > You mean the direct call into raa215300_rtc_probe_helper()? > > Yes, indeed. > > > I think that can be solved by enhancing i2c_new_ancillary_device() to take a > > proper device name, instead of using "dummy"? > > Geert, Wolfram, > > Any pointers for instantiating i2c_client device(for eg: rtc_isl1208.c) > by enhancing i2c_new_ancillary_device() api? > > Apart for this, since a single compatible(the pmic one) is used, > > In rtc device probe, How do we identify this PMIC RTC device and use the "TYPE_ISL1208" config > in RTC driver, once we instantiate RTC device? You create ancillary devices with different names, based on the detected PMIC version, and add new types for them to the isl1208 driver? Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Tuesday, May 9, 2023 10:40 AM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Lee Jones <lee@kernel.org>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Alessandro Zummo <a.zummo@towertech.it>; > Magnus Damm <magnus.damm@gmail.com>; linux-renesas-soc@vger.kernel.org; > Fabrizio Castro <fabrizio.castro.jz@renesas.com>; Wolfram Sang <wsa- > dev@sang-engineering.com> > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > Hi Biju, > > On Tue, May 9, 2023 at 11:32 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > driver On Tue, May 9, 2023 at 9:35 AM Biju Das > <biju.das.jz@bp.renesas.com> wrote: > > > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > > > driver On Tue, May 9, 2023 at 9:06 AM Biju Das > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > > > > > driver > > > > > > > > > > > > > > Currently, it is not possible to instantiate the i2c client > > > > > > > driver using MFD cell as it is not a platform driver. Add > > > > > > > support for Renesas PMIC > > > > > > > RAA215300 RTC platform driver, so that it can be > > > > > > > instantiated by MFD > > > > > API. > > > > > > > The rtc device is created by using > > > > > > > i2c_new_ancillary_device() and it register as the rtc device > > > > > > > by the helper function provided by > > > > > > > rtc-isl2108 driver. > > > > > > > > > > > > Not sure this platform driver has to be placed in RTC > > > > > > subsystem rather than MFD subsystem as PMIC MFD driver, can > > > > > > instantiate it using > > > MFD cell?? > > > > > > > > > > Can't you just instantiate the i2c ancillary device from the > > > > > PMIC driver, and drop the new mfd and platform rtc drivers? > > > > > > > > Yes, it is possible. > > > > > > > > The only reason MFD is used for future expansion like adding > > > > support for > > > > > > > > 1) battery charger > > > > Or > > > > 2) regulator > > > > > > I'd just start with a skeleton regulator ("PMIC") driver... > > > > > > > In that case, we can share regmap to sub devices. But these > > > > drivers are > > > not currently planned. > > > > > > > > Apart from that, > > > > > > > > 1) It avoids subsystem dependencies, ie, PMIC driver directly > > > > calling rtc > > > driver > > > > for registering RTC device. > > > > > > You mean the direct call into raa215300_rtc_probe_helper()? > > > > Yes, indeed. > > > > > I think that can be solved by enhancing i2c_new_ancillary_device() > > > to take a proper device name, instead of using "dummy"? > > > > Geert, Wolfram, > > > > Any pointers for instantiating i2c_client device(for eg: > > rtc_isl1208.c) by enhancing i2c_new_ancillary_device() api? > > > > Apart for this, since a single compatible(the pmic one) is used, > > > > In rtc device probe, How do we identify this PMIC RTC device and use > > the "TYPE_ISL1208" config in RTC driver, once we instantiate RTC device? > > You create ancillary devices with different names, based on the detected > PMIC version, and add new types for them to the isl1208 driver? OK, I have prototyped this. Wolfram, What is your opinion, should I enhance "i2c_new_ancillary_device" like below[1]? or directly call "i2c_new_client_device" from PMIC driver to instantiate the client device? Please let me know. [1] diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index ae3af738b03f..79808e852f2f 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1122,15 +1122,17 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device); * @client: Handle to the primary client * @name: Handle to specify which secondary address to get * @default_addr: Used as a fallback if no secondary address was specified + * @aux_device_name: Optional ancillary device name * Context: can sleep * * I2C clients can be composed of multiple I2C slaves bound together in a single * component. The I2C client driver then binds to the master I2C slave and needs - * to create I2C dummy clients to communicate with all the other slaves. + * to create I2C ancillary clients to communicate with all the other slaves. * - * This function creates and returns an I2C dummy client whose I2C address is - * retrieved from the platform firmware based on the given slave name. If no - * address is specified by the firmware default_addr is used. + * This function creates and returns an I2C ancillary client whose I2C address + * is retrieved from the platform firmware based on the given slave name. If no + * address is specified by the firmware default_addr is used. if no aux_device_ + * name is specified by the firmware, it will create i2c dummy client. * * On DT-based platforms the address is retrieved from the "reg" property entry * cell whose "reg-names" value matches the slave name. @@ -1139,10 +1141,12 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device); * i2c_unregister_device(); or an ERR_PTR to describe the error. */ struct i2c_client *i2c_new_ancillary_device(struct i2c_client *client, - const char *name, - u16 default_addr) + const char *name, + u16 default_addr, + const char *aux_device_name) { struct device_node *np = client->dev.of_node; + struct i2c_client *i2c_aux_client; u32 addr = default_addr; int i; @@ -1153,7 +1157,27 @@ struct i2c_client *i2c_new_ancillary_device(struct i2c_client *client, } dev_dbg(&client->adapter->dev, "Address for %s : 0x%x\n", name, addr); - return i2c_new_dummy_device(client->adapter, addr); + + if (aux_device_name) { + struct i2c_board_info info; + size_t device_name_len = strlen(device_name); + + if (aux_device_name_len > I2C_NAME_SIZE - 1) { + dev_err(&client->adapter->dev, "Invalid device name\n"); + return ERR_PTR(-EINVAL); + } + + memset(&info, 0, sizeof(struct i2c_board_info)); + + memcpy(info.type, device_name, device_name_len); + info.addr = addr; + + i2c_aux_client = i2c_new_client_device(client->adapter, &info); + } else { + i2c_aux_client = i2c_new_dummy_device(client->adapter, addr); + } + + return i2c_aux_client; } EXPORT_SYMBOL_GPL(i2c_new_ancillary_device); Cheers, Biju
Hi Wolfram, > -----Original Message----- > From: Biju Das <biju.das.jz@bp.renesas.com> > Sent: Tuesday, May 9, 2023 6:05 PM > To: Geert Uytterhoeven <geert@linux-m68k.org>; Wolfram Sang <wsa-dev@sang- > engineering.com> > Cc: Lee Jones <lee@kernel.org>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Alessandro Zummo <a.zummo@towertech.it>; > Magnus Damm <magnus.damm@gmail.com>; linux-renesas-soc@vger.kernel.org; > Fabrizio Castro <fabrizio.castro.jz@renesas.com> > Subject: RE: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > Hi Geert, > > Thanks for the feedback. > > > -----Original Message----- > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: Tuesday, May 9, 2023 10:40 AM > > To: Biju Das <biju.das.jz@bp.renesas.com> > > Cc: Lee Jones <lee@kernel.org>; Alexandre Belloni > > <alexandre.belloni@bootlin.com>; Alessandro Zummo > > <a.zummo@towertech.it>; Magnus Damm <magnus.damm@gmail.com>; > > linux-renesas-soc@vger.kernel.org; > > Fabrizio Castro <fabrizio.castro.jz@renesas.com>; Wolfram Sang <wsa- > > dev@sang-engineering.com> > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC driver > > > > Hi Biju, > > > > On Tue, May 9, 2023 at 11:32 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 RTC > > > > driver On Tue, May 9, 2023 at 9:35 AM Biju Das > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > Subject: Re: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 > > > > > > RTC driver On Tue, May 9, 2023 at 9:06 AM Biju Das > > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > > > Subject: [PATCH v2 4/5] mfd: Add Renesas PMIC RAA215300 > > > > > > > > RTC driver > > > > > > > > > > > > > > > > Currently, it is not possible to instantiate the i2c > > > > > > > > client driver using MFD cell as it is not a platform > > > > > > > > driver. Add support for Renesas PMIC > > > > > > > > RAA215300 RTC platform driver, so that it can be > > > > > > > > instantiated by MFD > > > > > > API. > > > > > > > > The rtc device is created by using > > > > > > > > i2c_new_ancillary_device() and it register as the rtc > > > > > > > > device by the helper function provided by > > > > > > > > rtc-isl2108 driver. > > > > > > > > > > > > > > Not sure this platform driver has to be placed in RTC > > > > > > > subsystem rather than MFD subsystem as PMIC MFD driver, can > > > > > > > instantiate it using > > > > MFD cell?? > > > > > > > > > > > > Can't you just instantiate the i2c ancillary device from the > > > > > > PMIC driver, and drop the new mfd and platform rtc drivers? > > > > > > > > > > Yes, it is possible. > > > > > > > > > > The only reason MFD is used for future expansion like adding > > > > > support for > > > > > > > > > > 1) battery charger > > > > > Or > > > > > 2) regulator > > > > > > > > I'd just start with a skeleton regulator ("PMIC") driver... > > > > > > > > > In that case, we can share regmap to sub devices. But these > > > > > drivers are > > > > not currently planned. > > > > > > > > > > Apart from that, > > > > > > > > > > 1) It avoids subsystem dependencies, ie, PMIC driver directly > > > > > calling rtc > > > > driver > > > > > for registering RTC device. > > > > > > > > You mean the direct call into raa215300_rtc_probe_helper()? > > > > > > Yes, indeed. > > > > > > > I think that can be solved by enhancing i2c_new_ancillary_device() > > > > to take a proper device name, instead of using "dummy"? > > > > > > Geert, Wolfram, > > > > > > Any pointers for instantiating i2c_client device(for eg: > > > rtc_isl1208.c) by enhancing i2c_new_ancillary_device() api? > > > > > > Apart for this, since a single compatible(the pmic one) is used, > > > > > > In rtc device probe, How do we identify this PMIC RTC device and use > > > the "TYPE_ISL1208" config in RTC driver, once we instantiate RTC device? > > > > You create ancillary devices with different names, based on the > > detected PMIC version, and add new types for them to the isl1208 driver? > > OK, I have prototyped this. > > Wolfram, > > What is your opinion, should I enhance "i2c_new_ancillary_device" like > below[1]? or directly call "i2c_new_client_device" from PMIC driver to > instantiate the client device? > > Please let me know. I will post a proper patch based on [1], so that we can discuss it there. Cheers, Biju > > [1] > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index > ae3af738b03f..79808e852f2f 100644 > --- a/drivers/i2c/i2c-core-base.c > +++ b/drivers/i2c/i2c-core-base.c > @@ -1122,15 +1122,17 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device); > * @client: Handle to the primary client > * @name: Handle to specify which secondary address to get > * @default_addr: Used as a fallback if no secondary address was specified > + * @aux_device_name: Optional ancillary device name > * Context: can sleep > * > * I2C clients can be composed of multiple I2C slaves bound together in a > single > * component. The I2C client driver then binds to the master I2C slave and > needs > - * to create I2C dummy clients to communicate with all the other slaves. > + * to create I2C ancillary clients to communicate with all the other > slaves. > * > - * This function creates and returns an I2C dummy client whose I2C address > is > - * retrieved from the platform firmware based on the given slave name. If > no > - * address is specified by the firmware default_addr is used. > + * This function creates and returns an I2C ancillary client whose I2C > + address > + * is retrieved from the platform firmware based on the given slave > + name. If no > + * address is specified by the firmware default_addr is used. if no > + aux_device_ > + * name is specified by the firmware, it will create i2c dummy client. > * > * On DT-based platforms the address is retrieved from the "reg" property > entry > * cell whose "reg-names" value matches the slave name. > @@ -1139,10 +1141,12 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device); > * i2c_unregister_device(); or an ERR_PTR to describe the error. > */ > struct i2c_client *i2c_new_ancillary_device(struct i2c_client *client, > - const char *name, > - u16 default_addr) > + const char *name, > + u16 default_addr, > + const char *aux_device_name) > { > struct device_node *np = client->dev.of_node; > + struct i2c_client *i2c_aux_client; > u32 addr = default_addr; > int i; > > @@ -1153,7 +1157,27 @@ struct i2c_client *i2c_new_ancillary_device(struct > i2c_client *client, > } > > dev_dbg(&client->adapter->dev, "Address for %s : 0x%x\n", name, addr); > - return i2c_new_dummy_device(client->adapter, addr); > + > + if (aux_device_name) { > + struct i2c_board_info info; > + size_t device_name_len = strlen(device_name); > + > + if (aux_device_name_len > I2C_NAME_SIZE - 1) { > + dev_err(&client->adapter->dev, "Invalid device name\n"); > + return ERR_PTR(-EINVAL); > + } > + > + memset(&info, 0, sizeof(struct i2c_board_info)); > + > + memcpy(info.type, device_name, device_name_len); > + info.addr = addr; > + > + i2c_aux_client = i2c_new_client_device(client->adapter, &info); > + } else { > + i2c_aux_client = i2c_new_dummy_device(client->adapter, addr); > + } > + > + return i2c_aux_client; > } > EXPORT_SYMBOL_GPL(i2c_new_ancillary_device); > > > Cheers, > Biju
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 9071b0f27b62..cec20c1f143d 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -318,6 +318,13 @@ config PMIC_RAA215300 help Support for the Renesas RAA215300 PMIC. +config PMIC_RAA215300_RTC + tristate "Renesas RAA215300 RTC driver" + depends on PMIC_RAA215300 + select RTC_DRV_ISL1208 + help + Select this to get support for the Renesas RAA215300 RTC + config PMIC_DA903X bool "Dialog Semiconductor DA9030/DA9034 PMIC Support" depends on I2C=y diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index d9c601120bfd..ed4b760e564e 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -127,6 +127,7 @@ obj-$(CONFIG_MCP_UCB1200) += ucb1x00-assabet.o endif obj-$(CONFIG_PMIC_RAA215300) += raa215300.o +obj-$(CONFIG_PMIC_RAA215300_RTC) += raa215300-rtc.o obj-$(CONFIG_PMIC_DA903X) += da903x.o diff --git a/drivers/mfd/raa215300-rtc.c b/drivers/mfd/raa215300-rtc.c new file mode 100644 index 000000000000..309ed34d6cd7 --- /dev/null +++ b/drivers/mfd/raa215300-rtc.c @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Renesas RAA215300 RTC Driver + * + * Copyright (C) 2023 Renesas Electronics Corporation + */ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/rtc/isl1208.h> + +#define RAA215300_RTC_DEFAULT_ADDR 0x6f + +static void raa215300_rtc_unregister_device(void *data) +{ + i2c_unregister_device(data); +} + +static int raa215300_rtc_probe(struct platform_device *pdev) +{ + unsigned int *pmic_version = dev_get_drvdata(pdev->dev.parent); + struct i2c_client *client; + int ret; + + client = i2c_new_ancillary_device(to_i2c_client(pdev->dev.parent), + "rtc", RAA215300_RTC_DEFAULT_ADDR); + if (IS_ERR(client)) + return PTR_ERR(client); + + ret = devm_add_action_or_reset(&pdev->dev, + raa215300_rtc_unregister_device, client); + if (ret < 0) + return ret; + + return raa215300_rtc_probe_helper(client, *pmic_version); +} + +static struct platform_driver raa215300_rtc_driver = { + .probe = raa215300_rtc_probe, + .driver = { + .name = "raa215300-rtc", + }, +}; +module_platform_driver(raa215300_rtc_driver); + +MODULE_DESCRIPTION("Renesas RAA215300 MFD RTC Driver"); +MODULE_ALIAS("platform:raa215300-rtc"); +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); +MODULE_LICENSE("GPL"); +MODULE_SOFTDEP("post: rtc_isl1208"); diff --git a/drivers/mfd/raa215300.c b/drivers/mfd/raa215300.c index 5cdd3213e99c..c8bdf96b203f 100644 --- a/drivers/mfd/raa215300.c +++ b/drivers/mfd/raa215300.c @@ -8,6 +8,7 @@ #include <linux/err.h> #include <linux/i2c.h> #include <linux/module.h> +#include <linux/mfd/core.h> #include <linux/of.h> #include <linux/regmap.h> @@ -29,13 +30,26 @@ static const struct regmap_config raa215300_regmap_config = { .cache_type = REGCACHE_FLAT, }; +static const struct mfd_cell raa215300_rtc_dev[] = { + { .name = "raa215300-rtc" } +}; + +static void raa215300_remove_devices(void *data) +{ + mfd_remove_devices(data); +} + static int raa215300_i2c_probe(struct i2c_client *client) { struct device *dev = &client->dev; - unsigned int pmic_version; + unsigned int *pmic_version; struct regmap *regmap; int ret; + pmic_version = devm_kzalloc(dev, sizeof(*pmic_version), GFP_KERNEL); + if (!pmic_version) + return -ENOMEM; + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) return -EOPNOTSUPP; @@ -44,11 +58,29 @@ static int raa215300_i2c_probe(struct i2c_client *client) return dev_err_probe(dev, PTR_ERR(regmap), "regmap i2c init failed\n"); - ret = regmap_read(regmap, RAA215300_HW_REV, &pmic_version); + ret = regmap_read(regmap, RAA215300_HW_REV, pmic_version); if (ret < 0) return dev_err_probe(dev, ret, "HW rev read failed\n"); - dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", pmic_version); + dev_dbg(dev, "RAA215300 PMIC version 0x%04x\n", *pmic_version); + dev_set_drvdata(dev, pmic_version); + + if (of_property_read_bool(dev->of_node, "renesas,rtc-enabled")) { + /* Enable RTC block */ + regmap_update_bits(regmap, RAA215300_REG_BLOCK_EN, + RAA215300_REG_BLOCK_EN_RTC_EN, + RAA215300_REG_BLOCK_EN_RTC_EN); + + ret = mfd_add_devices(dev, 0, raa215300_rtc_dev, + ARRAY_SIZE(raa215300_rtc_dev), NULL, + 0, NULL); + if (ret < 0) + return ret; + + ret = devm_add_action_or_reset(dev, raa215300_remove_devices, dev); + if (ret < 0) + return ret; + } return 0; }
Currently, it is not possible to instantiate the i2c client driver using MFD cell as it is not a platform driver. Add support for Renesas PMIC RAA215300 RTC platform driver, so that it can be instantiated by MFD API. The rtc device is created by using i2c_new_ancillary_device() and it register as the rtc device by the helper function provided by rtc-isl2108 driver. The PMIC driver enables RTC device and share the PMIC revision to RTC platform driver. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v2: * New patch --- drivers/mfd/Kconfig | 7 +++++ drivers/mfd/Makefile | 1 + drivers/mfd/raa215300-rtc.c | 52 +++++++++++++++++++++++++++++++++++++ drivers/mfd/raa215300.c | 38 ++++++++++++++++++++++++--- 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 drivers/mfd/raa215300-rtc.c