[PATCH] power: reset: linkstation-poweroff: add LS220D/E

[PATCH] power: reset: linkstation-poweroff: add LS220D/E

[Daniel González Cabanelas]

Add 2 new devices to the compatible list:
  - Buffalo Linkstation LS220D
  - Buffalo Linkstation LS220DE

Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
---
 drivers/power/reset/linkstation-poweroff.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/reset/linkstation-poweroff.c b/drivers/power/reset/linkstation-poweroff.c
index 02f5fdb8f..cfee2efd9 100644
--- a/drivers/power/reset/linkstation-poweroff.c
+++ b/drivers/power/reset/linkstation-poweroff.c
@@ -142,6 +142,12 @@ static void linkstation_poweroff(void)
 }
 
 static const struct of_device_id ls_poweroff_of_match[] = {
+	{ .compatible = "buffalo,ls220d",
+	  .data = &linkstation_power_off_cfg,
+	},
+	{ .compatible = "buffalo,ls220de",
+	  .data = &linkstation_power_off_cfg,
+	},
 	{ .compatible = "buffalo,ls421d",
 	  .data = &linkstation_power_off_cfg,
 	},
-- 
2.39.1

Re: [PATCH] power: reset: linkstation-poweroff: add LS220D/E

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

Hi,

On Sun, Feb 12, 2023 at 05:37:38PM +0100, Daniel González Cabanelas wrote:
> Add 2 new devices to the compatible list:
>   - Buffalo Linkstation LS220D
>   - Buffalo Linkstation LS220DE
> 
> Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> ---
>  drivers/power/reset/linkstation-poweroff.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/power/reset/linkstation-poweroff.c b/drivers/power/reset/linkstation-poweroff.c
> index 02f5fdb8f..cfee2efd9 100644
> --- a/drivers/power/reset/linkstation-poweroff.c
> +++ b/drivers/power/reset/linkstation-poweroff.c
> @@ -142,6 +142,12 @@ static void linkstation_poweroff(void)
>  }
>  
>  static const struct of_device_id ls_poweroff_of_match[] = {
> +	{ .compatible = "buffalo,ls220d",
> +	  .data = &linkstation_power_off_cfg,
> +	},
> +	{ .compatible = "buffalo,ls220de",
> +	  .data = &linkstation_power_off_cfg,
> +	},
>  	{ .compatible = "buffalo,ls421d",
>  	  .data = &linkstation_power_off_cfg,
>  	},

Where is the patch adding these compatibles to the DT binding
documentation?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] power: reset: linkstation-poweroff: add LS220D/E

[Daniel González Cabanelas]

Hi

El lun, 13 feb 2023 a las 21:25, Sebastian Reichel
(<sebastian.reichel@collabora.com>) escribió:
>
> Hi,
>
> On Sun, Feb 12, 2023 at 05:37:38PM +0100, Daniel González Cabanelas wrote:
> > Add 2 new devices to the compatible list:
> >   - Buffalo Linkstation LS220D
> >   - Buffalo Linkstation LS220DE
> >
> > Signed-off-by: Daniel González Cabanelas <dgcbueu@gmail.com>
> > ---
> >  drivers/power/reset/linkstation-poweroff.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/power/reset/linkstation-poweroff.c b/drivers/power/reset/linkstation-poweroff.c
> > index 02f5fdb8f..cfee2efd9 100644
> > --- a/drivers/power/reset/linkstation-poweroff.c
> > +++ b/drivers/power/reset/linkstation-poweroff.c
> > @@ -142,6 +142,12 @@ static void linkstation_poweroff(void)
> >  }
> >
> >  static const struct of_device_id ls_poweroff_of_match[] = {
> > +     { .compatible = "buffalo,ls220d",
> > +       .data = &linkstation_power_off_cfg,
> > +     },
> > +     { .compatible = "buffalo,ls220de",
> > +       .data = &linkstation_power_off_cfg,
> > +     },
> >       { .compatible = "buffalo,ls421d",
> >         .data = &linkstation_power_off_cfg,
> >       },
>
> Where is the patch adding these compatibles to the DT binding
> documentation?

There is no DT binding at all. So no documentation.

BTW I'm thinking to rework the driver again for adding DT bindings and
therefore rid of the dependeny of adding new devices to the driver,
every time a new compatible device appears. My first versions of this
driver had DT-bindings but the DT documentation maintainer rejected
them. He said there was no need to add any DT binding. So we ended
with this no sense. Let's see if the next time we have a bit more
luck.

Regards


>
> -- Sebastian

Re: [PATCH] power: reset: linkstation-poweroff: add LS220D/E

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

[+cc DT binding people]

Hi,

On Mon, Feb 13, 2023 at 09:38:24PM +0100, Daniel González Cabanelas wrote:
> > >  static const struct of_device_id ls_poweroff_of_match[] = {
> > > +     { .compatible = "buffalo,ls220d",
> > > +       .data = &linkstation_power_off_cfg,
> > > +     },
> > > +     { .compatible = "buffalo,ls220de",
> > > +       .data = &linkstation_power_off_cfg,
> > > +     },
> > >       { .compatible = "buffalo,ls421d",
> > >         .data = &linkstation_power_off_cfg,
> > >       },
> >
> > Where is the patch adding these compatibles to the DT binding
> > documentation?
> 
> There is no DT binding at all. So no documentation.

You are referencing a compatible, so there is supposed to be
a DT binding for it. Note, that you also need DT bindings for
board level compatible values. See for example:

Documentation/devicetree/bindings/arm/rockchip.yaml
Documentation/devicetree/bindings/arm/fsl.yaml

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] power: reset: linkstation-poweroff: add LS220D/E

[Daniel González Cabanelas]

Hi Sebastian,

El lun, 13 feb 2023 a las 22:22, Sebastian Reichel
(<sebastian.reichel@collabora.com>) escribió:
>
> [+cc DT binding people]
>
> Hi,
>
> On Mon, Feb 13, 2023 at 09:38:24PM +0100, Daniel González Cabanelas wrote:
> > > >  static const struct of_device_id ls_poweroff_of_match[] = {
> > > > +     { .compatible = "buffalo,ls220d",
> > > > +       .data = &linkstation_power_off_cfg,
> > > > +     },
> > > > +     { .compatible = "buffalo,ls220de",
> > > > +       .data = &linkstation_power_off_cfg,
> > > > +     },
> > > >       { .compatible = "buffalo,ls421d",
> > > >         .data = &linkstation_power_off_cfg,
> > > >       },
> > >
> > > Where is the patch adding these compatibles to the DT binding
> > > documentation?
> >
> > There is no DT binding at all. So no documentation.
>
> You are referencing a compatible, so there is supposed to be
> a DT binding for it. Note, that you also need DT bindings for
> board level compatible values. See for example:
>
> Documentation/devicetree/bindings/arm/rockchip.yaml
> Documentation/devicetree/bindings/arm/fsl.yaml

Since the driver uses the root compatible string, I don't see any
binding to document at least for the driver itself.  Nor I don't see
where a reference for this driver should be put if I documented the
board compatible strings.

>
> -- Sebastian

Re: [PATCH] power: reset: linkstation-poweroff: add LS220D/E

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1853 bytes --]

Hi,

On Mon, Feb 27, 2023 at 09:17:39PM +0100, Daniel González Cabanelas wrote:
> El lun, 13 feb 2023 a las 22:22, Sebastian Reichel
> (<sebastian.reichel@collabora.com>) escribió:
> > [+cc DT binding people]
> >
> > Hi,
> >
> > On Mon, Feb 13, 2023 at 09:38:24PM +0100, Daniel González Cabanelas wrote:
> > > > >  static const struct of_device_id ls_poweroff_of_match[] = {
> > > > > +     { .compatible = "buffalo,ls220d",
> > > > > +       .data = &linkstation_power_off_cfg,
> > > > > +     },
> > > > > +     { .compatible = "buffalo,ls220de",
> > > > > +       .data = &linkstation_power_off_cfg,
> > > > > +     },
> > > > >       { .compatible = "buffalo,ls421d",
> > > > >         .data = &linkstation_power_off_cfg,
> > > > >       },
> > > >
> > > > Where is the patch adding these compatibles to the DT binding
> > > > documentation?
> > >
> > > There is no DT binding at all. So no documentation.
> >
> > You are referencing a compatible, so there is supposed to be
> > a DT binding for it. Note, that you also need DT bindings for
> > board level compatible values. See for example:
> >
> > Documentation/devicetree/bindings/arm/rockchip.yaml
> > Documentation/devicetree/bindings/arm/fsl.yaml
> 
> Since the driver uses the root compatible string, I don't see any
> binding to document at least for the driver itself. Nor I don't see
> where a reference for this driver should be put if I documented the
> board compatible strings.

You should document the board compatible string for the board
(ignoring this driver). Actually that should have happened
before the board DT has been merged in the first place. Note,
that the examples I provided above are for boards.

Since you are only referencing the root compatible string, we
are good to go afterwards from DT perspective.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]