Patchwork [1/5] rtc: s5m: Fix register updating by adding regmap for RTC

login
register
mail settings
Submitter Krzysztof Kozlowski
Date Nov. 26, 2013, 1:50 p.m.
Message ID <1385473828-31078-2-git-send-email-k.kozlowski@samsung.com>
Download mbox | patch
Permalink /patch/294330/
State New
Headers show

Comments

Krzysztof Kozlowski - Nov. 26, 2013, 1:50 p.m.
On S5M8767A registers were not properly updated and read due to usage of
the same regmap as the PMIC. This could be observed in various hangs,
e.g. in infinite loop during waiting for UDR field change.

On this chip family the RTC has different I2C address than PMIC so
additional regmap is needed.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/sec-core.c           |   14 ++++++++++++++
 drivers/rtc/rtc-s5m.c            |    2 +-
 include/linux/mfd/samsung/core.h |    1 +
 3 files changed, 16 insertions(+), 1 deletion(-)
Geert Uytterhoeven - Nov. 26, 2013, 2:07 p.m.
On Tue, Nov 26, 2013 at 2:50 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On S5M8767A registers were not properly updated and read due to usage of
> the same regmap as the PMIC. This could be observed in various hangs,
> e.g. in infinite loop during waiting for UDR field change.
>
> On this chip family the RTC has different I2C address than PMIC so
> additional regmap is needed.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mfd/sec-core.c           |   14 ++++++++++++++
>  drivers/rtc/rtc-s5m.c            |    2 +-
>  include/linux/mfd/samsung/core.h |    1 +
>  3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 34c18fb..a1a413a 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -166,6 +166,11 @@ static struct regmap_config s5m8767_regmap_config = {
>         .cache_type = REGCACHE_FLAT,
>  };
>
> +static struct regmap_config sec_rtc_regmap_config = {

const please

> --- a/include/linux/mfd/samsung/core.h
> +++ b/include/linux/mfd/samsung/core.h
> @@ -40,6 +40,7 @@ struct sec_pmic_dev {
>         struct device *dev;
>         struct sec_platform_data *pdata;
>         struct regmap *regmap;
> +       struct regmap *regmap_rtc;

Do you think it makes sense to rename the plain "regmap" to "regmap_pmic"?

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
Krzysztof Kozlowski - Nov. 26, 2013, 2:26 p.m.
On Tue, 2013-11-26 at 15:07 +0100, Geert Uytterhoeven wrote:
> On Tue, Nov 26, 2013 at 2:50 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > On S5M8767A registers were not properly updated and read due to usage of
> > the same regmap as the PMIC. This could be observed in various hangs,
> > e.g. in infinite loop during waiting for UDR field change.
> >
> > On this chip family the RTC has different I2C address than PMIC so
> > additional regmap is needed.
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/mfd/sec-core.c           |   14 ++++++++++++++
> >  drivers/rtc/rtc-s5m.c            |    2 +-
> >  include/linux/mfd/samsung/core.h |    1 +
> >  3 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> > index 34c18fb..a1a413a 100644
> > --- a/drivers/mfd/sec-core.c
> > +++ b/drivers/mfd/sec-core.c
> > @@ -166,6 +166,11 @@ static struct regmap_config s5m8767_regmap_config = {
> >         .cache_type = REGCACHE_FLAT,
> >  };
> >
> > +static struct regmap_config sec_rtc_regmap_config = {
> 
> const please

Sure.

> > --- a/include/linux/mfd/samsung/core.h
> > +++ b/include/linux/mfd/samsung/core.h
> > @@ -40,6 +40,7 @@ struct sec_pmic_dev {
> >         struct device *dev;
> >         struct sec_platform_data *pdata;
> >         struct regmap *regmap;
> > +       struct regmap *regmap_rtc;
> 
> Do you think it makes sense to rename the plain "regmap" to "regmap_pmic"?

Yes, I think it would make the code more readable. I'll change it.

Best regards,
Krzysztof

Patch

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 34c18fb..a1a413a 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -166,6 +166,11 @@  static struct regmap_config s5m8767_regmap_config = {
 	.cache_type = REGCACHE_FLAT,
 };
 
+static struct regmap_config sec_rtc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
 #ifdef CONFIG_OF
 /*
  * Only the common platform data elements for s5m8767 are parsed here from the
@@ -277,6 +282,15 @@  static int sec_pmic_probe(struct i2c_client *i2c,
 	sec_pmic->rtc = i2c_new_dummy(i2c->adapter, RTC_I2C_ADDR);
 	i2c_set_clientdata(sec_pmic->rtc, sec_pmic);
 
+	sec_pmic->regmap_rtc = devm_regmap_init_i2c(sec_pmic->rtc,
+			&sec_rtc_regmap_config);
+	if (IS_ERR(sec_pmic->regmap_rtc)) {
+		ret = PTR_ERR(sec_pmic->regmap_rtc);
+		dev_err(&i2c->dev, "Failed to allocate RTC register map: %d\n",
+			ret);
+		return ret;
+	}
+
 	if (pdata && pdata->cfg_pmic_irq)
 		pdata->cfg_pmic_irq();
 
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index 1dfa488..088cf3a 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -542,7 +542,7 @@  static int s5m_rtc_probe(struct platform_device *pdev)
 
 	info->dev = &pdev->dev;
 	info->s5m87xx = s5m87xx;
-	info->regmap = s5m87xx->regmap;
+	info->regmap = s5m87xx->regmap_rtc;
 	info->device_type = s5m87xx->device_type;
 	info->wtsr_smpl = s5m87xx->wtsr_smpl;
 
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 2d0c907..692acad 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -40,6 +40,7 @@  struct sec_pmic_dev {
 	struct device *dev;
 	struct sec_platform_data *pdata;
 	struct regmap *regmap;
+	struct regmap *regmap_rtc;
 	struct i2c_client *i2c;
 	struct i2c_client *rtc;