| Message ID | 20230503084608.14008-6-biju.das.jz@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
On 03/05/2023 09:46:07+0100, Biju Das wrote: > The built-in RTC found on PMIC RAA215300 is the same as ISL1208. > However, the external oscillator polarity is determined by the > PMIC version. For eg: the PMIC version has inverted polarity for > the external oscillator and the corresponding bit in RTC need to > be inverted(XTOSCB). This info needs to be shared from PMIC driver > to RTC driver, so that it can support all versions without any code > changes. > > Add a new compatible renesas,raa215300-isl1208 to support RTC found > on PMIC RAA215300 and renesas,raa215300-pmic property to support > different versions. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > drivers/rtc/rtc-isl1208.c | 50 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c > index 73cc6aaf9b8b..f4ea19691ac1 100644 > --- a/drivers/rtc/rtc-isl1208.c > +++ b/drivers/rtc/rtc-isl1208.c > @@ -74,6 +74,7 @@ enum isl1208_id { > TYPE_ISL1209, > TYPE_ISL1218, > TYPE_ISL1219, > + TYPE_RAA215300_ISL1208, > ISL_LAST_ID > }; > > @@ -83,11 +84,13 @@ static const struct isl1208_config { > unsigned int nvmem_length; > unsigned has_tamper:1; > unsigned has_timestamp:1; > + unsigned has_pmic_parent:1; > } isl1208_configs[] = { > [TYPE_ISL1208] = { "isl1208", 2, false, false }, > [TYPE_ISL1209] = { "isl1209", 2, true, false }, > [TYPE_ISL1218] = { "isl1218", 8, false, false }, > [TYPE_ISL1219] = { "isl1219", 2, true, true }, > + [TYPE_RAA215300_ISL1208] = { "isl1208", 2, false, false, true }, > }; > > static const struct i2c_device_id isl1208_id[] = { > @@ -104,6 +107,10 @@ static const __maybe_unused struct of_device_id isl1208_of_match[] = { > { .compatible = "isil,isl1209", .data = &isl1208_configs[TYPE_ISL1209] }, > { .compatible = "isil,isl1218", .data = &isl1208_configs[TYPE_ISL1218] }, > { .compatible = "isil,isl1219", .data = &isl1208_configs[TYPE_ISL1219] }, > + { > + .compatible = "renesas,raa215300-isl1208", > + .data = &isl1208_configs[TYPE_RAA215300_ISL1208] > + }, > { } > }; > MODULE_DEVICE_TABLE(of, isl1208_of_match); > @@ -166,6 +173,43 @@ isl1208_i2c_validate_client(struct i2c_client *client) > return 0; > } > > +static bool isl1208_is_xtosc_polarity_inverted(struct i2c_client *client) I'd remove polarity from the name of this function > +{ > + struct device *dev = &client->dev; > + struct i2c_client *pmic_dev; > + unsigned int *pmic_version; > + struct device_node *np; > + bool ret = false; > + > + np = of_parse_phandle(dev->of_node, "renesas,raa215300-pmic", 0); > + if (np) > + pmic_dev = of_find_i2c_device_by_node(np); > + > + of_node_put(np); > + if (!pmic_dev) > + return ret; > + > + pmic_version = dev_get_drvdata(&pmic_dev->dev); > + /* External Oscillator polarity is inverted on revision 0x12 onwards */ s/polarity/bit/ My understanding is that the bit meaning is inverted. It is still a on/off bit. > + if (*pmic_version >= 0x12) > + ret = true; > + > + put_device(&pmic_dev->dev); > + > + return ret; > +} > + > +static int > +isl1208_set_ext_osc_based_on_pmic_version(struct i2c_client *client, int rc) > +{ > + if (isl1208_is_xtosc_polarity_inverted(client)) > + rc &= ~ISL1208_REG_SR_XTOSCB; > + else > + rc |= ISL1208_REG_SR_XTOSCB; > + > + return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, rc); > +} > + > static int > isl1208_i2c_get_sr(struct i2c_client *client) > { > @@ -845,6 +889,12 @@ isl1208_probe(struct i2c_client *client) > return rc; > } > > + if (isl1208->config->has_pmic_parent) { > + rc = isl1208_set_ext_osc_based_on_pmic_version(client, rc); > + if (rc) > + return rc; > + } > + > if (rc & ISL1208_REG_SR_RTCF) > dev_warn(&client->dev, "rtc power failure detected, " > "please set clock.\n"); > -- > 2.25.1 >
Hi Alexandre Belloni, Thanks for the feedback. > Subject: Re: [PATCH RFC 5/6] rtc: isl1208: Add support for the built-in RTC > on the PMIC RAA215300 > > On 03/05/2023 09:46:07+0100, Biju Das wrote: > > The built-in RTC found on PMIC RAA215300 is the same as ISL1208. > > However, the external oscillator polarity is determined by the PMIC > > version. For eg: the PMIC version has inverted polarity for the > > external oscillator and the corresponding bit in RTC need to be > > inverted(XTOSCB). This info needs to be shared from PMIC driver to RTC > > driver, so that it can support all versions without any code changes. > > > > Add a new compatible renesas,raa215300-isl1208 to support RTC found on > > PMIC RAA215300 and renesas,raa215300-pmic property to support > > different versions. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > drivers/rtc/rtc-isl1208.c | 50 > > +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 50 insertions(+) > > > > diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c > > index 73cc6aaf9b8b..f4ea19691ac1 100644 > > --- a/drivers/rtc/rtc-isl1208.c > > +++ b/drivers/rtc/rtc-isl1208.c > > @@ -74,6 +74,7 @@ enum isl1208_id { > > TYPE_ISL1209, > > TYPE_ISL1218, > > TYPE_ISL1219, > > + TYPE_RAA215300_ISL1208, > > ISL_LAST_ID > > }; > > > > @@ -83,11 +84,13 @@ static const struct isl1208_config { > > unsigned int nvmem_length; > > unsigned has_tamper:1; > > unsigned has_timestamp:1; > > + unsigned has_pmic_parent:1; > > } isl1208_configs[] = { > > [TYPE_ISL1208] = { "isl1208", 2, false, false }, > > [TYPE_ISL1209] = { "isl1209", 2, true, false }, > > [TYPE_ISL1218] = { "isl1218", 8, false, false }, > > [TYPE_ISL1219] = { "isl1219", 2, true, true }, > > + [TYPE_RAA215300_ISL1208] = { "isl1208", 2, false, false, true }, > > }; > > > > static const struct i2c_device_id isl1208_id[] = { @@ -104,6 +107,10 > > @@ static const __maybe_unused struct of_device_id isl1208_of_match[] = { > > { .compatible = "isil,isl1209", .data = &isl1208_configs[TYPE_ISL1209] > }, > > { .compatible = "isil,isl1218", .data = &isl1208_configs[TYPE_ISL1218] > }, > > { .compatible = "isil,isl1219", .data = > > &isl1208_configs[TYPE_ISL1219] }, > > + { > > + .compatible = "renesas,raa215300-isl1208", > > + .data = &isl1208_configs[TYPE_RAA215300_ISL1208] > > + }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, isl1208_of_match); @@ -166,6 +173,43 @@ > > isl1208_i2c_validate_client(struct i2c_client *client) > > return 0; > > } > > > > +static bool isl1208_is_xtosc_polarity_inverted(struct i2c_client > > +*client) > > I'd remove polarity from the name of this function Agreed. > > > +{ > > + struct device *dev = &client->dev; > > + struct i2c_client *pmic_dev; > > + unsigned int *pmic_version; > > + struct device_node *np; > > + bool ret = false; > > + > > + np = of_parse_phandle(dev->of_node, "renesas,raa215300-pmic", 0); > > + if (np) > > + pmic_dev = of_find_i2c_device_by_node(np); > > + > > + of_node_put(np); > > + if (!pmic_dev) > > + return ret; > > + > > + pmic_version = dev_get_drvdata(&pmic_dev->dev); > > + /* External Oscillator polarity is inverted on revision 0x12 onwards > > +*/ > > s/polarity/bit/ > > My understanding is that the bit meaning is inverted. It is still a on/off > bit. Yes, that is correct bit is inverted. PMIC version 0x11 ----------------- bit 0: Disable internal crystal oscillator 1: Enable internal crystal oscillator PMIC version 0x12 ---------------- bit 0: Enable internal crystal oscillator 1: Disable internal crystal oscillator Cheers, Biju > > > + if (*pmic_version >= 0x12) > > + ret = true; > > + > > + put_device(&pmic_dev->dev); > > + > > + return ret; > > +} > > + > > +static int > > +isl1208_set_ext_osc_based_on_pmic_version(struct i2c_client *client, > > +int rc) { > > + if (isl1208_is_xtosc_polarity_inverted(client)) > > + rc &= ~ISL1208_REG_SR_XTOSCB; > > + else > > + rc |= ISL1208_REG_SR_XTOSCB; > > + > > + return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, rc); } > > + > > static int > > isl1208_i2c_get_sr(struct i2c_client *client) { @@ -845,6 +889,12 @@ > > isl1208_probe(struct i2c_client *client) > > return rc; > > } > > > > + if (isl1208->config->has_pmic_parent) { > > + rc = isl1208_set_ext_osc_based_on_pmic_version(client, rc); > > + if (rc) > > + return rc; > > + } > > + > > if (rc & ISL1208_REG_SR_RTCF) > > dev_warn(&client->dev, "rtc power failure detected, " > > "please set clock.\n"); > > -- > > 2.25.1 > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.co/ > m%2F&data=05%7C01%7Cbiju.das.jz%40bp.renesas.com%7C68856b4bc4f14d42055608db4 > bc51ccd%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638187082188797517%7CUn > known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX > VCI6Mn0%3D%7C3000%7C%7C%7C&sdata=mzGMSc1wg7XjQ97oMyCzQZ6UEHbuViyvOErFefp8pF0 > %3D&reserved=0
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c index 73cc6aaf9b8b..f4ea19691ac1 100644 --- a/drivers/rtc/rtc-isl1208.c +++ b/drivers/rtc/rtc-isl1208.c @@ -74,6 +74,7 @@ enum isl1208_id { TYPE_ISL1209, TYPE_ISL1218, TYPE_ISL1219, + TYPE_RAA215300_ISL1208, ISL_LAST_ID }; @@ -83,11 +84,13 @@ static const struct isl1208_config { unsigned int nvmem_length; unsigned has_tamper:1; unsigned has_timestamp:1; + unsigned has_pmic_parent:1; } isl1208_configs[] = { [TYPE_ISL1208] = { "isl1208", 2, false, false }, [TYPE_ISL1209] = { "isl1209", 2, true, false }, [TYPE_ISL1218] = { "isl1218", 8, false, false }, [TYPE_ISL1219] = { "isl1219", 2, true, true }, + [TYPE_RAA215300_ISL1208] = { "isl1208", 2, false, false, true }, }; static const struct i2c_device_id isl1208_id[] = { @@ -104,6 +107,10 @@ static const __maybe_unused struct of_device_id isl1208_of_match[] = { { .compatible = "isil,isl1209", .data = &isl1208_configs[TYPE_ISL1209] }, { .compatible = "isil,isl1218", .data = &isl1208_configs[TYPE_ISL1218] }, { .compatible = "isil,isl1219", .data = &isl1208_configs[TYPE_ISL1219] }, + { + .compatible = "renesas,raa215300-isl1208", + .data = &isl1208_configs[TYPE_RAA215300_ISL1208] + }, { } }; MODULE_DEVICE_TABLE(of, isl1208_of_match); @@ -166,6 +173,43 @@ isl1208_i2c_validate_client(struct i2c_client *client) return 0; } +static bool isl1208_is_xtosc_polarity_inverted(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct i2c_client *pmic_dev; + unsigned int *pmic_version; + struct device_node *np; + bool ret = false; + + np = of_parse_phandle(dev->of_node, "renesas,raa215300-pmic", 0); + if (np) + pmic_dev = of_find_i2c_device_by_node(np); + + of_node_put(np); + if (!pmic_dev) + return ret; + + pmic_version = dev_get_drvdata(&pmic_dev->dev); + /* External Oscillator polarity is inverted on revision 0x12 onwards */ + if (*pmic_version >= 0x12) + ret = true; + + put_device(&pmic_dev->dev); + + return ret; +} + +static int +isl1208_set_ext_osc_based_on_pmic_version(struct i2c_client *client, int rc) +{ + if (isl1208_is_xtosc_polarity_inverted(client)) + rc &= ~ISL1208_REG_SR_XTOSCB; + else + rc |= ISL1208_REG_SR_XTOSCB; + + return i2c_smbus_write_byte_data(client, ISL1208_REG_SR, rc); +} + static int isl1208_i2c_get_sr(struct i2c_client *client) { @@ -845,6 +889,12 @@ isl1208_probe(struct i2c_client *client) return rc; } + if (isl1208->config->has_pmic_parent) { + rc = isl1208_set_ext_osc_based_on_pmic_version(client, rc); + if (rc) + return rc; + } + if (rc & ISL1208_REG_SR_RTCF) dev_warn(&client->dev, "rtc power failure detected, " "please set clock.\n");
The built-in RTC found on PMIC RAA215300 is the same as ISL1208. However, the external oscillator polarity is determined by the PMIC version. For eg: the PMIC version has inverted polarity for the external oscillator and the corresponding bit in RTC need to be inverted(XTOSCB). This info needs to be shared from PMIC driver to RTC driver, so that it can support all versions without any code changes. Add a new compatible renesas,raa215300-isl1208 to support RTC found on PMIC RAA215300 and renesas,raa215300-pmic property to support different versions. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/rtc/rtc-isl1208.c | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)