[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[Fabrizio Castro]

Not every driver initialises of_node from struct gpio_chip,
therefore the replacement of of_node from struct gpio_chip
with dev->of_node in the below commit won't work on every
platform:
baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' property")
The final result is that on some platforms the kernel will
try to dereference a NULL pointer, with obvious consequences.

This patch makes sure the pointer gets initialised before its
first usage.

Fixes: baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' property")
Reported-by: Johnson CH Chen <JohnsonCH.Chen@moxa.com>
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* v1 was for testing purposes only, but no point in sending
  out a dirty patch, therefore cleaned it up
---
 drivers/gpio/gpiolib-of.c | 2 +-
 drivers/gpio/gpiolib.c    | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index ec642bf..6fa1818 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -338,7 +338,7 @@ static void of_gpiochip_init_valid_mask(struct gpio_chip *chip)
 {
 	int len, i;
 	u32 start, count;
-	struct device_node *np = chip->dev->of_node;
+	struct device_node *np = chip->of_node;
 
 	len = of_property_count_u32_elems(np,  "gpio-reserved-ranges");
 	if (len < 0 || len % 2 != 0)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d72218f..820e3e3d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -296,7 +296,7 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gpiochip)
 {
 #ifdef CONFIG_OF_GPIO
 	int size;
-	struct device_node *np = gpiochip->dev->of_node;
+	struct device_node *np = gpiochip->of_node;
 
 	size = of_property_count_u32_elems(np,  "gpio-reserved-ranges");
 	if (size > 0 && size % 2 == 0)
@@ -360,6 +360,9 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
 
 	chip->data = data;
 
+	if ((!chip->of_node) && (chip->dev))
+		chip->of_node = chip->dev->of_node;
+
 	spin_lock_irqsave(&gpio_lock, flags);
 
 	if (base < 0) {
-- 
2.7.4

[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[Fabrizio Castro]

Hi guys,

We have got a good feedback from Johnson, what do you think
about this patch?

Thanks,
Fab

> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Sent: 08 November 2019 16:47
> Subject: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer
> 
> Not every driver initialises of_node from struct gpio_chip,
> therefore the replacement of of_node from struct gpio_chip
> with dev->of_node in the below commit won't work on every
> platform:
> baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' property")
> The final result is that on some platforms the kernel will
> try to dereference a NULL pointer, with obvious consequences.
> 
> This patch makes sure the pointer gets initialised before its
> first usage.
> 
> Fixes: baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' property")
> Reported-by: Johnson CH Chen <JohnsonCH.Chen@moxa.com>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * v1 was for testing purposes only, but no point in sending
>   out a dirty patch, therefore cleaned it up
> ---
>  drivers/gpio/gpiolib-of.c | 2 +-
>  drivers/gpio/gpiolib.c    | 5 ++++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index ec642bf..6fa1818 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -338,7 +338,7 @@ static void of_gpiochip_init_valid_mask(struct gpio_chip *chip)
>  {
>  	int len, i;
>  	u32 start, count;
> -	struct device_node *np = chip->dev->of_node;
> +	struct device_node *np = chip->of_node;
> 
>  	len = of_property_count_u32_elems(np,  "gpio-reserved-ranges");
>  	if (len < 0 || len % 2 != 0)
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d72218f..820e3e3d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -296,7 +296,7 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gpiochip)
>  {
>  #ifdef CONFIG_OF_GPIO
>  	int size;
> -	struct device_node *np = gpiochip->dev->of_node;
> +	struct device_node *np = gpiochip->of_node;
> 
>  	size = of_property_count_u32_elems(np,  "gpio-reserved-ranges");
>  	if (size > 0 && size % 2 == 0)
> @@ -360,6 +360,9 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data)
> 
>  	chip->data = data;
> 
> +	if ((!chip->of_node) && (chip->dev))
> +		chip->of_node = chip->dev->of_node;
> +
>  	spin_lock_irqsave(&gpio_lock, flags);
> 
>  	if (base < 0) {
> --
> 2.7.4

[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[nobuhiro1.iwamatsu at toshiba.co.jp]

Hi,

> -----Original Message-----
> From: Fabrizio Castro [mailto:fabrizio.castro at bp.renesas.com]
> Sent: Monday, November 11, 2019 6:49 PM
> To: iwamatsu nobuhiro(?? ?? ????????)
> <nobuhiro1.iwamatsu@toshiba.co.jp>; pavel at denx.de
> Cc: JohnsonCH.Chen at moxa.com; cip-dev at lists.cip-project.org; Fabrizio
> Castro <fabrizio.castro@bp.renesas.com>; Chris Paterson
> <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>
> Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad
> of_node pointer
> 
> Hi guys,
> 
> We have got a good feedback from Johnson, what do you think about this
> patch?
> 
> Thanks,
> Fab
> 
> > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > Sent: 08 November 2019 16:47
> > Subject: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad
> > of_node pointer
> >
> > Not every driver initialises of_node from struct gpio_chip, therefore
> > the replacement of of_node from struct gpio_chip with dev->of_node in
> > the below commit won't work on every
> > platform:
> > baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' property") The
> > final result is that on some platforms the kernel will try to
> > dereference a NULL pointer, with obvious consequences.
> >
> > This patch makes sure the pointer gets initialised before its first
> > usage.
> >
> > Fixes: baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges'
> > property")
> > Reported-by: Johnson CH Chen <JohnsonCH.Chen@moxa.com>
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v1->v2:
> > * v1 was for testing purposes only, but no point in sending
> >   out a dirty patch, therefore cleaned it up
> > ---
> >  drivers/gpio/gpiolib-of.c | 2 +-
> >  drivers/gpio/gpiolib.c    | 5 ++++-
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> > index ec642bf..6fa1818 100644
> > --- a/drivers/gpio/gpiolib-of.c
> > +++ b/drivers/gpio/gpiolib-of.c
> > @@ -338,7 +338,7 @@ static void of_gpiochip_init_valid_mask(struct
> > gpio_chip *chip)  {
> >  	int len, i;
> >  	u32 start, count;
> > -	struct device_node *np = chip->dev->of_node;
> > +	struct device_node *np = chip->of_node;
> >
> >  	len = of_property_count_u32_elems(np,
> "gpio-reserved-ranges");
> >  	if (len < 0 || len % 2 != 0)
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index
> > d72218f..820e3e3d 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -296,7 +296,7 @@ static int gpiochip_init_valid_mask(struct
> > gpio_chip *gpiochip)  {  #ifdef CONFIG_OF_GPIO
> >  	int size;
> > -	struct device_node *np = gpiochip->dev->of_node;
> > +	struct device_node *np = gpiochip->of_node;
> >

This already changed by commit 726cb3ba49692bdae6caff457755e7cdb432efa4.
If we apply this, we need to revert and update commit baff4777cdb80.

> >  	size = of_property_count_u32_elems(np,
> "gpio-reserved-ranges");
> >  	if (size > 0 && size % 2 == 0)
> > @@ -360,6 +360,9 @@ int gpiochip_add_data(struct gpio_chip *chip, void
> > *data)
> >
> >  	chip->data = data;
> >
> > +	if ((!chip->of_node) && (chip->dev))
> > +		chip->of_node = chip->dev->of_node;
> > +
> >  	spin_lock_irqsave(&gpio_lock, flags);
> >
> >  	if (base < 0) {


I think this is a good patch to solve the problem.
However, as a CIP, this patch that is not in upstream cannot be imported.
I think we need to apply the following patches (and others).

acc6e331b62275570d23b20ced6296812023967f
6ff0497402ef7269ee6a72f62eb85adaa7a4768e

> > --
> > 2.7.4

Best regards,
  Nobuhiro

[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[Fabrizio Castro]

Hello Iwamatsu-san,

Thank you for your feedback!

> From: nobuhiro1.iwamatsu at toshiba.co.jp <nobuhiro1.iwamatsu@toshiba.co.jp>
> Sent: 12 November 2019 00:30
> Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer
> 
> Hi,
> 
> > -----Original Message-----
> > From: Fabrizio Castro [mailto:fabrizio.castro at bp.renesas.com]
> > Sent: Monday, November 11, 2019 6:49 PM
> > To: iwamatsu nobuhiro(?? ?? ????????)
> > <nobuhiro1.iwamatsu@toshiba.co.jp>; pavel at denx.de
> > Cc: JohnsonCH.Chen at moxa.com; cip-dev at lists.cip-project.org; Fabrizio
> > Castro <fabrizio.castro@bp.renesas.com>; Chris Paterson
> > <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>
> > Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad
> > of_node pointer
> >
> > Hi guys,
> >
> > We have got a good feedback from Johnson, what do you think about this
> > patch?
> >
> > Thanks,
> > Fab
> >
> > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > Sent: 08 November 2019 16:47
> > > Subject: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad
> > > of_node pointer
> > >
> > > Not every driver initialises of_node from struct gpio_chip, therefore
> > > the replacement of of_node from struct gpio_chip with dev->of_node in
> > > the below commit won't work on every
> > > platform:
> > > baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' property") The
> > > final result is that on some platforms the kernel will try to
> > > dereference a NULL pointer, with obvious consequences.
> > >
> > > This patch makes sure the pointer gets initialised before its first
> > > usage.
> > >
> > > Fixes: baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges'
> > > property")
> > > Reported-by: Johnson CH Chen <JohnsonCH.Chen@moxa.com>
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > >
> > > ---
> > > v1->v2:
> > > * v1 was for testing purposes only, but no point in sending
> > >   out a dirty patch, therefore cleaned it up
> > > ---
> > >  drivers/gpio/gpiolib-of.c | 2 +-
> > >  drivers/gpio/gpiolib.c    | 5 ++++-
> > >  2 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> > > index ec642bf..6fa1818 100644
> > > --- a/drivers/gpio/gpiolib-of.c
> > > +++ b/drivers/gpio/gpiolib-of.c
> > > @@ -338,7 +338,7 @@ static void of_gpiochip_init_valid_mask(struct
> > > gpio_chip *chip)  {
> > >  	int len, i;
> > >  	u32 start, count;
> > > -	struct device_node *np = chip->dev->of_node;
> > > +	struct device_node *np = chip->of_node;
> > >
> > >  	len = of_property_count_u32_elems(np,
> > "gpio-reserved-ranges");
> > >  	if (len < 0 || len % 2 != 0)
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index
> > > d72218f..820e3e3d 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -296,7 +296,7 @@ static int gpiochip_init_valid_mask(struct
> > > gpio_chip *gpiochip)  {  #ifdef CONFIG_OF_GPIO
> > >  	int size;
> > > -	struct device_node *np = gpiochip->dev->of_node;
> > > +	struct device_node *np = gpiochip->of_node;
> > >
> 
> This already changed by commit 726cb3ba49692bdae6caff457755e7cdb432efa4.
> If we apply this, we need to revert and update commit baff4777cdb80.
> 
> > >  	size = of_property_count_u32_elems(np,
> > "gpio-reserved-ranges");
> > >  	if (size > 0 && size % 2 == 0)
> > > @@ -360,6 +360,9 @@ int gpiochip_add_data(struct gpio_chip *chip, void
> > > *data)
> > >
> > >  	chip->data = data;
> > >
> > > +	if ((!chip->of_node) && (chip->dev))
> > > +		chip->of_node = chip->dev->of_node;
> > > +
> > >  	spin_lock_irqsave(&gpio_lock, flags);
> > >
> > >  	if (base < 0) {
> 
> 
> I think this is a good patch to solve the problem.
> However, as a CIP, this patch that is not in upstream cannot be imported.
> I think we need to apply the following patches (and others).
> 
> acc6e331b62275570d23b20ced6296812023967f
> 6ff0497402ef7269ee6a72f62eb85adaa7a4768e

All those patches were done in the context of gpiochip being real devices,
but that only happened in v4.6, please see ff2b1359229927563addbf2f5ad480660c350903

I did try going down the backporting route before coming up with a brand new patch,
but unfortunately the context of the upstream patches is not right for this due
to the fact that we don't have gpiochip devices support in v4.4.y-cip gpiolib.

Thanks,
Fab

> 
> > > --
> > > 2.7.4
> 
> Best regards,
>   Nobuhiro

[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[Johnson CH Chen (陳昭勳)]

Hi,
>
> Hello Iwamatsu-san,
>
> Thank you for your feedback!
>
> > From: nobuhiro1.iwamatsu at toshiba.co.jp 
> > <nobuhiro1.iwamatsu@toshiba.co.jp>
> > Sent: 12 November 2019 00:30
> > Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad 
> > of_node pointer
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Fabrizio Castro [mailto:fabrizio.castro at bp.renesas.com]
> > > Sent: Monday, November 11, 2019 6:49 PM
> > > To: iwamatsu nobuhiro(?? ?? ????????)
> > > <nobuhiro1.iwamatsu@toshiba.co.jp>; pavel at denx.de
> > > Cc: JohnsonCH.Chen at moxa.com; cip-dev at lists.cip-project.org; 
> > > Fabrizio Castro <fabrizio.castro@bp.renesas.com>; Chris Paterson 
> > > <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>
> > > Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix 
> > > bad of_node pointer
> > >
> > > Hi guys,
> > >
> > > We have got a good feedback from Johnson, what do you think about 
> > > this patch?
> > >
> > > Thanks,
> > > Fab
> > >
> > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > Sent: 08 November 2019 16:47
> > > > Subject: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad 
> > > > of_node pointer
> > > >
> > > > Not every driver initialises of_node from struct gpio_chip, 
> > > > therefore the replacement of of_node from struct gpio_chip with
> > > > dev->of_node in the below commit won't work on every
> > > > platform:
> > > > baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges' 
> > > > property") The final result is that on some platforms the kernel 
> > > > will try to dereference a NULL pointer, with obvious consequences.
> > > >
> > > > This patch makes sure the pointer gets initialised before its 
> > > > first usage.
> > > >
> > > > Fixes: baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges'
> > > > property")
> > > > Reported-by: Johnson CH Chen <JohnsonCH.Chen@moxa.com>
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > >
> > > > ---
> > > > v1->v2:
> > > > * v1 was for testing purposes only, but no point in sending
> > > >   out a dirty patch, therefore cleaned it up
> > > > ---
> > > >  drivers/gpio/gpiolib-of.c | 2 +-
> > > >  drivers/gpio/gpiolib.c    | 5 ++++-
> > > >  2 files changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpio/gpiolib-of.c 
> > > > b/drivers/gpio/gpiolib-of.c index ec642bf..6fa1818 100644
> > > > --- a/drivers/gpio/gpiolib-of.c
> > > > +++ b/drivers/gpio/gpiolib-of.c
> > > > @@ -338,7 +338,7 @@ static void 
> > > > of_gpiochip_init_valid_mask(struct
> > > > gpio_chip *chip)  {
> > > >   int len, i;
> > > >   u32 start, count;
> > > > - struct device_node *np = chip->dev->of_node;
> > > > + struct device_node *np = chip->of_node;
> > > >
> > > >   len = of_property_count_u32_elems(np,
> > > "gpio-reserved-ranges");
> > > >   if (len < 0 || len % 2 != 0)
> > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c 
> > > > index d72218f..820e3e3d 100644
> > > > --- a/drivers/gpio/gpiolib.c
> > > > +++ b/drivers/gpio/gpiolib.c
> > > > @@ -296,7 +296,7 @@ static int gpiochip_init_valid_mask(struct 
> > > > gpio_chip *gpiochip)  {  #ifdef CONFIG_OF_GPIO
> > > >   int size;
> > > > - struct device_node *np = gpiochip->dev->of_node;
> > > > + struct device_node *np = gpiochip->of_node;
> > > >
> >
> > This already changed by commit 726cb3ba49692bdae6caff457755e7cdb432efa4.
> > If we apply this, we need to revert and update commit baff4777cdb80.
> >
> > > >   size = of_property_count_u32_elems(np,
> > > "gpio-reserved-ranges");
> > > >   if (size > 0 && size % 2 == 0) @@ -360,6 +360,9 @@ int 
> > > > gpiochip_add_data(struct gpio_chip *chip, void
> > > > *data)
> > > >
> > > >   chip->data = data;
> > > >
> > > > + if ((!chip->of_node) && (chip->dev))
> > > > +         chip->of_node = chip->dev->of_node;
> > > > +
> > > >   spin_lock_irqsave(&gpio_lock, flags);
> > > >
> > > >   if (base < 0) {
> >
> >
> > I think this is a good patch to solve the problem.
> > However, as a CIP, this patch that is not in upstream cannot be imported.
> > I think we need to apply the following patches (and others).
> >
> > acc6e331b62275570d23b20ced6296812023967f
> > 6ff0497402ef7269ee6a72f62eb85adaa7a4768e
>
> All those patches were done in the context of gpiochip being real 
> devices, but that only happened in v4.6, please see 
> ff2b1359229927563addbf2f5ad480660c350903
>

Is that mean it's better to apply the following patches from v4.6 into v4.4.y-cip to fix the problem?
"acc6e331b62275570d23b20ced6296812023967f"
"6ff0497402ef7269ee6a72f62eb85adaa7a4768e"
"ff2b1359229927563addbf2f5ad480660c350903"


> I did try going down the backporting route before coming up with a brand new patch, but unfortunately the context of the upstream patches is not right for this due to the fact that we don't have gpiochip devices support in v4.4.y-cip gpiolib.
>
> Thanks,
> Fab
>
> >
> > > > --
> > > > 2.7.4
> >
> > Best regards,
> >   Nobuhiro

Thanks,
Johnson

[cip-dev] [RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer

[Fabrizio Castro]

Hi Johnson,

Thank you for your email!

> From: Johnson CH Chen (???) <JohnsonCH.Chen@moxa.com>
> Sent: 13 November 2019 09:17
> Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad of_node pointer
> 
> Hi,
> >
> > Hello Iwamatsu-san,
> >
> > Thank you for your feedback!
> >
> > > From: nobuhiro1.iwamatsu at toshiba.co.jp
> > > <nobuhiro1.iwamatsu@toshiba.co.jp>
> > > Sent: 12 November 2019 00:30
> > > Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad
> > > of_node pointer
> > >
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Fabrizio Castro [mailto:fabrizio.castro at bp.renesas.com]
> > > > Sent: Monday, November 11, 2019 6:49 PM
> > > > To: iwamatsu nobuhiro(?? ?? ????????)
> > > > <nobuhiro1.iwamatsu@toshiba.co.jp>; pavel at denx.de
> > > > Cc: JohnsonCH.Chen at moxa.com; cip-dev at lists.cip-project.org;
> > > > Fabrizio Castro <fabrizio.castro@bp.renesas.com>; Chris Paterson
> > > > <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>
> > > > Subject: RE: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix
> > > > bad of_node pointer
> > > >
> > > > Hi guys,
> > > >
> > > > We have got a good feedback from Johnson, what do you think about
> > > > this patch?
> > > >
> > > > Thanks,
> > > > Fab
> > > >
> > > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > Sent: 08 November 2019 16:47
> > > > > Subject: [cip-dev][RFT/RFC linux-4.4.y-cip v2] gpiolib: Fix bad
> > > > > of_node pointer
> > > > >
> > > > > Not every driver initialises of_node from struct gpio_chip,
> > > > > therefore the replacement of of_node from struct gpio_chip with
> > > > > dev->of_node in the below commit won't work on every
> > > > > platform:
> > > > > baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges'
> > > > > property") The final result is that on some platforms the kernel
> > > > > will try to dereference a NULL pointer, with obvious consequences.
> > > > >
> > > > > This patch makes sure the pointer gets initialised before its
> > > > > first usage.
> > > > >
> > > > > Fixes: baff4777cdb8 ("gpiolib: Support 'gpio-reserved-ranges'
> > > > > property")
> > > > > Reported-by: Johnson CH Chen <JohnsonCH.Chen@moxa.com>
> > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > >
> > > > > ---
> > > > > v1->v2:
> > > > > * v1 was for testing purposes only, but no point in sending
> > > > >   out a dirty patch, therefore cleaned it up
> > > > > ---
> > > > >  drivers/gpio/gpiolib-of.c | 2 +-
> > > > >  drivers/gpio/gpiolib.c    | 5 ++++-
> > > > >  2 files changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpio/gpiolib-of.c
> > > > > b/drivers/gpio/gpiolib-of.c index ec642bf..6fa1818 100644
> > > > > --- a/drivers/gpio/gpiolib-of.c
> > > > > +++ b/drivers/gpio/gpiolib-of.c
> > > > > @@ -338,7 +338,7 @@ static void
> > > > > of_gpiochip_init_valid_mask(struct
> > > > > gpio_chip *chip)  {
> > > > >   int len, i;
> > > > >   u32 start, count;
> > > > > - struct device_node *np = chip->dev->of_node;
> > > > > + struct device_node *np = chip->of_node;
> > > > >
> > > > >   len = of_property_count_u32_elems(np,
> > > > "gpio-reserved-ranges");
> > > > >   if (len < 0 || len % 2 != 0)
> > > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > > > index d72218f..820e3e3d 100644
> > > > > --- a/drivers/gpio/gpiolib.c
> > > > > +++ b/drivers/gpio/gpiolib.c
> > > > > @@ -296,7 +296,7 @@ static int gpiochip_init_valid_mask(struct
> > > > > gpio_chip *gpiochip)  {  #ifdef CONFIG_OF_GPIO
> > > > >   int size;
> > > > > - struct device_node *np = gpiochip->dev->of_node;
> > > > > + struct device_node *np = gpiochip->of_node;
> > > > >
> > >
> > > This already changed by commit 726cb3ba49692bdae6caff457755e7cdb432efa4.
> > > If we apply this, we need to revert and update commit baff4777cdb80.
> > >
> > > > >   size = of_property_count_u32_elems(np,
> > > > "gpio-reserved-ranges");
> > > > >   if (size > 0 && size % 2 == 0) @@ -360,6 +360,9 @@ int
> > > > > gpiochip_add_data(struct gpio_chip *chip, void
> > > > > *data)
> > > > >
> > > > >   chip->data = data;
> > > > >
> > > > > + if ((!chip->of_node) && (chip->dev))
> > > > > +         chip->of_node = chip->dev->of_node;
> > > > > +
> > > > >   spin_lock_irqsave(&gpio_lock, flags);
> > > > >
> > > > >   if (base < 0) {
> > >
> > >
> > > I think this is a good patch to solve the problem.
> > > However, as a CIP, this patch that is not in upstream cannot be imported.
> > > I think we need to apply the following patches (and others).
> > >
> > > acc6e331b62275570d23b20ced6296812023967f
> > > 6ff0497402ef7269ee6a72f62eb85adaa7a4768e
> >
> > All those patches were done in the context of gpiochip being real
> > devices, but that only happened in v4.6, please see
> > ff2b1359229927563addbf2f5ad480660c350903
> >
> 
> Is that mean it's better to apply the following patches from v4.6 into v4.4.y-cip to fix the problem?
> "acc6e331b62275570d23b20ced6296812023967f"
> "6ff0497402ef7269ee6a72f62eb85adaa7a4768e"
> "ff2b1359229927563addbf2f5ad480660c350903"

With the CIP project we tend to stay away from the subsystems and generic code as much as possible.
In the context of the patch that broke your platform we had no other choice because the RZ/G1C has an hole in its GPIOs layout, but as you have noticed touching generic code may destabilize the kernel.
Backporting those commits would mean touching code we shouldn't be touching in this project, and therefore I think it's a no go, I rather we fixed the patch that broke the kernel in the first place than introducing more code that may destabilize the kernel even more. I don't think we should be adding gpiochip devices support to 4.4, unless we really, really, really have to.

Thanks,
Fab

> 
> 
> > I did try going down the backporting route before coming up with a brand new patch, but unfortunately the context of the upstream
> patches is not right for this due to the fact that we don't have gpiochip devices support in v4.4.y-cip gpiolib.
> >
> > Thanks,
> > Fab
> >
> > >
> > > > > --
> > > > > 2.7.4
> > >
> > > Best regards,
> > >   Nobuhiro
> 
> Thanks,
> Johnson