[PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Simon Horman]

From: Wolfram Sang 

The current implementation only works if the da9xxx devices are added
before their drivers are registered. Only then it can apply the fixes to
both devices. Otherwise, the driver for the first device gets probed
before the fix for the second device can be applied. This is what
fails when using the IP core switcher or when having the i2c master
driver as a module.

So, we need to disable both da9xxx once we detected one of them. We now
use i2c_transfer with hardcoded i2c_messages and device addresses, so we
don't need the da9xxx client devices to be instantiated. Because the
fixup is used on specific boards only, the addresses are not going to
change.

Fixes: 663fbb52159cca ("ARM: shmobile: R-Car Gen2: Add da9063/da9210 regulator 
quirk")
Signed-off-by: Wolfram Sang 
Reviewed-by: Geert Uytterhoeven 
Tested-by: Geert Uytterhoeven  (r8a7791/koelsch)
Tested-by: Kuninori Morimoto 
Signed-off-by: Simon Horman 
---
 arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c | 62 +-
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c 
b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
index 62437b57813e..73e3adbc1330 100644
--- a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
@@ -41,39 +41,26 @@
 
 #define REGULATOR_IRQ_MASK BIT(2)  /* IRQ2, active low */
 
-static void __iomem *irqc;
-
-static const u8 da9063_mask_regs[] = {
-   DA9063_REG_IRQ_MASK_A,
-   DA9063_REG_IRQ_MASK_B,
-   DA9063_REG_IRQ_MASK_C,
-   DA9063_REG_IRQ_MASK_D,
-};
-
-/* DA9210 System Control and Event Registers */
+/* start of DA9210 System Control and Event Registers */
 #define DA9210_REG_MASK_A  0x54
-#define DA9210_REG_MASK_B  0x55
-
-static const u8 da9210_mask_regs[] = {
-   DA9210_REG_MASK_A,
-   DA9210_REG_MASK_B,
-};
-
-static void da9xxx_mask_irqs(struct i2c_client *client, const u8 regs[],
-unsigned int nregs)
-{
-   unsigned int i;
 
-   dev_info(>dev, "Masking %s interrupt sources\n", client->name);
+static void __iomem *irqc;
 
-   for (i = 0; i < nregs; i++) {
-   int error = i2c_smbus_write_byte_data(client, regs[i], ~0);
-   if (error) {
-   dev_err(>dev, "i2c error %d\n", error);
-   return;
-   }
-   }
-}
+/* first byte sets the memory pointer, following are consecutive reg values */
+static u8 da9063_irq_clr[] = { DA9063_REG_IRQ_MASK_A, 0xff, 0xff, 0xff, 0xff };
+static u8 da9210_irq_clr[] = { DA9210_REG_MASK_A, 0xff, 0xff };
+
+static struct i2c_msg da9xxx_msgs[2] = {
+   {
+   .addr = 0x58,
+   .len = ARRAY_SIZE(da9063_irq_clr),
+   .buf = da9063_irq_clr,
+   }, {
+   .addr = 0x68,
+   .len = ARRAY_SIZE(da9210_irq_clr),
+   .buf = da9210_irq_clr,
+   },
+};
 
 static int regulator_quirk_notify(struct notifier_block *nb,
  unsigned long action, void *data)
@@ -93,12 +80,15 @@ static int regulator_quirk_notify(struct notifier_block *nb,
client = to_i2c_client(dev);
dev_dbg(dev, "Detected %s\n", client->name);
 
-   if ((client->addr == 0x58 && !strcmp(client->name, "da9063")))
-   da9xxx_mask_irqs(client, da9063_mask_regs,
-ARRAY_SIZE(da9063_mask_regs));
-   else if (client->addr == 0x68 && !strcmp(client->name, "da9210"))
-   da9xxx_mask_irqs(client, da9210_mask_regs,
-ARRAY_SIZE(da9210_mask_regs));
+   if ((client->addr == 0x58 && !strcmp(client->name, "da9063")) ||
+   (client->addr == 0x68 && !strcmp(client->name, "da9210"))) {
+   int ret;
+
+   dev_info(>dev, "clearing da9063/da9210 interrupts\n");
+   ret = i2c_transfer(client->adapter, da9xxx_msgs, 
ARRAY_SIZE(da9xxx_msgs));
+   if (ret != ARRAY_SIZE(da9xxx_msgs))
+   dev_err(>dev, "i2c error %d\n", ret);
+   }
 
mon = ioread32(irqc + IRQC_MONITOR);
if (mon & REGULATOR_IRQ_MASK)
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Simon Horman]

On Thu, Sep 01, 2016 at 09:00:07PM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 1, 2016 at 4:27 PM, Wolfram Sang  wrote:
> > On Thu, Sep 01, 2016 at 04:22:37PM +0200, Simon Horman wrote:
> >> On Tue, Aug 30, 2016 at 05:15:58PM +0200, Geert Uytterhoeven wrote:
> >> > On Tue, Aug 30, 2016 at 4:54 PM, Wolfram Sang
> >> >  wrote:
> >> > > The current implementation only works if the da9xxx devices are added
> >> > > before their drivers are registered. Only then it can apply the fixes 
> >> > > to
> >> > > both devices. Otherwise, the driver for the first device gets probed
> >> > > before the fix for the second device can be applied. This is what
> >> > > fails when using the IP core switcher or when having the i2c master
> >> > > driver as a module.
> >> >
> >> > > So, we need to disable both da9xxx once we detected one of them. We now
> >> > > use i2c_transfer with hardcoded i2c_messages and device addresses, so 
> >> > > we
> >> > > don't need the da9xxx client devices to be instantiated. Because the
> >> > > fixup is used on specific boards only, the addresses are not going to
> >> > > change.
> >> > >
> >> > > Signed-off-by: Wolfram Sang 
> >> >
> >> > Reviewed-by: Geert Uytterhoeven 
> >> > Tested-by: Geert Uytterhoeven  (r8a7791/koelsch)
> >>
> >> Can I get a Fixes tag or some indication of if it should apply
> >> to v4.9 or as a fix for v4.8.
> >
> > I'd say:
> >
> > Fixes: 663fbb52159cca ("ARM: shmobile: R-Car Gen2: Add da9063/da9210 
> > regulator quirk")
> >
> > and should go to 4.8. Geert?
> 
> That's right. CC stable, for v4.1+ and any LTS(I) it's been backported to.

Thanks, got it.

Re: [PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Geert Uytterhoeven]

On Thu, Sep 1, 2016 at 4:27 PM, Wolfram Sang  wrote:
> On Thu, Sep 01, 2016 at 04:22:37PM +0200, Simon Horman wrote:
>> On Tue, Aug 30, 2016 at 05:15:58PM +0200, Geert Uytterhoeven wrote:
>> > On Tue, Aug 30, 2016 at 4:54 PM, Wolfram Sang
>> >  wrote:
>> > > The current implementation only works if the da9xxx devices are added
>> > > before their drivers are registered. Only then it can apply the fixes to
>> > > both devices. Otherwise, the driver for the first device gets probed
>> > > before the fix for the second device can be applied. This is what
>> > > fails when using the IP core switcher or when having the i2c master
>> > > driver as a module.
>> >
>> > > So, we need to disable both da9xxx once we detected one of them. We now
>> > > use i2c_transfer with hardcoded i2c_messages and device addresses, so we
>> > > don't need the da9xxx client devices to be instantiated. Because the
>> > > fixup is used on specific boards only, the addresses are not going to
>> > > change.
>> > >
>> > > Signed-off-by: Wolfram Sang 
>> >
>> > Reviewed-by: Geert Uytterhoeven 
>> > Tested-by: Geert Uytterhoeven  (r8a7791/koelsch)
>>
>> Can I get a Fixes tag or some indication of if it should apply
>> to v4.9 or as a fix for v4.8.
>
> I'd say:
>
> Fixes: 663fbb52159cca ("ARM: shmobile: R-Car Gen2: Add da9063/da9210 
> regulator quirk")
>
> and should go to 4.8. Geert?

That's right. CC stable, for v4.1+ and any LTS(I) it's been backported to.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

Re: [PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Simon Horman]

On Tue, Aug 30, 2016 at 05:15:58PM +0200, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Tue, Aug 30, 2016 at 4:54 PM, Wolfram Sang
>  wrote:
> > The current implementation only works if the da9xxx devices are added
> > before their drivers are registered. Only then it can apply the fixes to
> > both devices. Otherwise, the driver for the first device gets probed
> > before the fix for the second device can be applied. This is what
> > fails when using the IP core switcher or when having the i2c master
> > driver as a module.
> 
> Thanks a lot!
> That could also happen when irqc is a victim of deferred probing.
> 
> > So, we need to disable both da9xxx once we detected one of them. We now
> > use i2c_transfer with hardcoded i2c_messages and device addresses, so we
> > don't need the da9xxx client devices to be instantiated. Because the
> > fixup is used on specific boards only, the addresses are not going to
> > change.
> >
> > Signed-off-by: Wolfram Sang 
> 
> Reviewed-by: Geert Uytterhoeven 
> Tested-by: Geert Uytterhoeven  (r8a7791/koelsch)

Can I get a Fixes tag or some indication of if it should apply
to v4.9 or as a fix for v4.8.

Re: [PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Wolfram Sang]

On Thu, Sep 01, 2016 at 04:22:37PM +0200, Simon Horman wrote:
> On Tue, Aug 30, 2016 at 05:15:58PM +0200, Geert Uytterhoeven wrote:
> > Hi Wolfram,
> > 
> > On Tue, Aug 30, 2016 at 4:54 PM, Wolfram Sang
> >  wrote:
> > > The current implementation only works if the da9xxx devices are added
> > > before their drivers are registered. Only then it can apply the fixes to
> > > both devices. Otherwise, the driver for the first device gets probed
> > > before the fix for the second device can be applied. This is what
> > > fails when using the IP core switcher or when having the i2c master
> > > driver as a module.
> > 
> > Thanks a lot!
> > That could also happen when irqc is a victim of deferred probing.
> > 
> > > So, we need to disable both da9xxx once we detected one of them. We now
> > > use i2c_transfer with hardcoded i2c_messages and device addresses, so we
> > > don't need the da9xxx client devices to be instantiated. Because the
> > > fixup is used on specific boards only, the addresses are not going to
> > > change.
> > >
> > > Signed-off-by: Wolfram Sang 
> > 
> > Reviewed-by: Geert Uytterhoeven 
> > Tested-by: Geert Uytterhoeven  (r8a7791/koelsch)
> 
> Can I get a Fixes tag or some indication of if it should apply
> to v4.9 or as a fix for v4.8.

I'd say:

Fixes: 663fbb52159cca ("ARM: shmobile: R-Car Gen2: Add da9063/da9210 regulator 
quirk")

and should go to 4.8. Geert?



signature.asc
Description: PGP signature

Re: [PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Aug 30, 2016 at 4:54 PM, Wolfram Sang
 wrote:
> The current implementation only works if the da9xxx devices are added
> before their drivers are registered. Only then it can apply the fixes to
> both devices. Otherwise, the driver for the first device gets probed
> before the fix for the second device can be applied. This is what
> fails when using the IP core switcher or when having the i2c master
> driver as a module.

Thanks a lot!
That could also happen when irqc is a victim of deferred probing.

> So, we need to disable both da9xxx once we detected one of them. We now
> use i2c_transfer with hardcoded i2c_messages and device addresses, so we
> don't need the da9xxx client devices to be instantiated. Because the
> fixup is used on specific boards only, the addresses are not going to
> change.
>
> Signed-off-by: Wolfram Sang 

Reviewed-by: Geert Uytterhoeven 
Tested-by: Geert Uytterhoeven  (r8a7791/koelsch)

> --- a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
> @@ -41,39 +41,25 @@
>
>  #define REGULATOR_IRQ_MASK BIT(2)  /* IRQ2, active low */
>
> -static void __iomem *irqc;
> -
> -static const u8 da9063_mask_regs[] = {
> -   DA9063_REG_IRQ_MASK_A,
> -   DA9063_REG_IRQ_MASK_B,
> -   DA9063_REG_IRQ_MASK_C,
> -   DA9063_REG_IRQ_MASK_D,
> -};

At first I was puzzled by the removal of all the _B, _C, and _C registers...

> -
> -/* DA9210 System Control and Event Registers */
> +/* start of DA9210 System Control and Event Registers */
>  #define DA9210_REG_MASK_A  0x54
> -#define DA9210_REG_MASK_B  0x55
> -
> -static const u8 da9210_mask_regs[] = {
> -   DA9210_REG_MASK_A,
> -   DA9210_REG_MASK_B,
> -};

> +static u8 da9063_irq_clr[] = { DA9063_REG_IRQ_MASK_A, 0xff, 0xff, 0xff, 0xff 
> };
> +static u8 da9210_irq_clr[] = { DA9210_REG_MASK_A, 0xff, 0xff };

... so you may want to add a comment that _A 'till _D resp. _A and _B are
cleared.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

[PATCH] ARM: shmobile: fix regulator quirk for Gen2

[Wolfram Sang]

The current implementation only works if the da9xxx devices are added
before their drivers are registered. Only then it can apply the fixes to
both devices. Otherwise, the driver for the first device gets probed
before the fix for the second device can be applied. This is what
fails when using the IP core switcher or when having the i2c master
driver as a module.

So, we need to disable both da9xxx once we detected one of them. We now
use i2c_transfer with hardcoded i2c_messages and device addresses, so we
don't need the da9xxx client devices to be instantiated. Because the
fixup is used on specific boards only, the addresses are not going to
change.

Signed-off-by: Wolfram Sang 
---
 arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c | 61 +-
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c 
b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
index 62437b57813eab..23c84e09490d00 100644
--- a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
@@ -41,39 +41,25 @@
 
 #define REGULATOR_IRQ_MASK BIT(2)  /* IRQ2, active low */
 
-static void __iomem *irqc;
-
-static const u8 da9063_mask_regs[] = {
-   DA9063_REG_IRQ_MASK_A,
-   DA9063_REG_IRQ_MASK_B,
-   DA9063_REG_IRQ_MASK_C,
-   DA9063_REG_IRQ_MASK_D,
-};
-
-/* DA9210 System Control and Event Registers */
+/* start of DA9210 System Control and Event Registers */
 #define DA9210_REG_MASK_A  0x54
-#define DA9210_REG_MASK_B  0x55
-
-static const u8 da9210_mask_regs[] = {
-   DA9210_REG_MASK_A,
-   DA9210_REG_MASK_B,
-};
-
-static void da9xxx_mask_irqs(struct i2c_client *client, const u8 regs[],
-unsigned int nregs)
-{
-   unsigned int i;
 
-   dev_info(>dev, "Masking %s interrupt sources\n", client->name);
+static void __iomem *irqc;
 
-   for (i = 0; i < nregs; i++) {
-   int error = i2c_smbus_write_byte_data(client, regs[i], ~0);
-   if (error) {
-   dev_err(>dev, "i2c error %d\n", error);
-   return;
-   }
-   }
-}
+static u8 da9063_irq_clr[] = { DA9063_REG_IRQ_MASK_A, 0xff, 0xff, 0xff, 0xff };
+static u8 da9210_irq_clr[] = { DA9210_REG_MASK_A, 0xff, 0xff };
+
+static struct i2c_msg da9xxx_msgs[2] = {
+   {
+   .addr = 0x58,
+   .len = ARRAY_SIZE(da9063_irq_clr),
+   .buf = da9063_irq_clr,
+   }, {
+   .addr = 0x68,
+   .len = ARRAY_SIZE(da9210_irq_clr),
+   .buf = da9210_irq_clr,
+   },
+};
 
 static int regulator_quirk_notify(struct notifier_block *nb,
  unsigned long action, void *data)
@@ -93,12 +79,15 @@ static int regulator_quirk_notify(struct notifier_block *nb,
client = to_i2c_client(dev);
dev_dbg(dev, "Detected %s\n", client->name);
 
-   if ((client->addr == 0x58 && !strcmp(client->name, "da9063")))
-   da9xxx_mask_irqs(client, da9063_mask_regs,
-ARRAY_SIZE(da9063_mask_regs));
-   else if (client->addr == 0x68 && !strcmp(client->name, "da9210"))
-   da9xxx_mask_irqs(client, da9210_mask_regs,
-ARRAY_SIZE(da9210_mask_regs));
+   if ((client->addr == 0x58 && !strcmp(client->name, "da9063")) ||
+   (client->addr == 0x68 && !strcmp(client->name, "da9210"))) {
+   int ret;
+
+   dev_info(>dev, "clearing da9063/da9210 interrupts\n");
+   ret = i2c_transfer(client->adapter, da9xxx_msgs, 
ARRAY_SIZE(da9xxx_msgs));
+   if (ret != ARRAY_SIZE(da9xxx_msgs))
+   dev_err(>dev, "i2c error %d\n", ret);
+   }
 
mon = ioread32(irqc + IRQC_MONITOR);
if (mon & REGULATOR_IRQ_MASK)
-- 
2.9.3