diff mbox

[v3,1/5] clk: sunxi: Add apb0 gates for H3

Message ID 1454542430-16572-2-git-send-email-k@japko.eu
State New
Headers show

Commit Message

Krzysztof Adamski Feb. 3, 2016, 11:33 p.m. UTC
This patch adds support for APB0 in H3. It seems to be compatible with
earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
etc).

Signed-off-by: Krzysztof Adamski <k@japko.eu>
---
 Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
 drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
 2 files changed, 3 insertions(+)

Comments

Jean-Francois Moine Feb. 4, 2016, 2:47 p.m. UTC | #1
On Thu,  4 Feb 2016 00:33:46 +0100
Krzysztof Adamski <k@japko.eu> wrote:

> This patch adds support for APB0 in H3. It seems to be compatible with
> earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
> etc).
> 
> Signed-off-by: Krzysztof Adamski <k@japko.eu>
> ---
>  Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
>  drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index e59f57b..751c8b9f0 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -45,6 +45,7 @@ Required properties:
>  	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
>  	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
>  	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
> +	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
>  	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
>  	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
>  	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
> diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
> index f4da52b..6753c87 100644
> --- a/drivers/clk/sunxi/clk-simple-gates.c
> +++ b/drivers/clk/sunxi/clk-simple-gates.c
> @@ -130,6 +130,8 @@ CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
>  	       sunxi_simple_gates_init);
>  CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
>  	       sunxi_simple_gates_init);
> +CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
> +	       sunxi_simple_gates_init);
>  CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
>  	       sunxi_simple_gates_init);
>  CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",

It seems that the other compatible strings are there for historical
reasons. Why do you need a new one with such a specific name?

It would have been more sensible to add a generic compatible string as
"allwinner,apb-gates", letting the removal of the other strings for a
later patch...
Krzysztof Adamski Feb. 4, 2016, 8:56 p.m. UTC | #2
On Thu, Feb 04, 2016 at 03:47:52PM +0100, Jean-Francois Moine wrote:
>On Thu,  4 Feb 2016 00:33:46 +0100
>Krzysztof Adamski <k@japko.eu> wrote:
>
>> This patch adds support for APB0 in H3. It seems to be compatible with
>> earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
>> etc).
>>
>> Signed-off-by: Krzysztof Adamski <k@japko.eu>
>> ---
>>  Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
>>  drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> index e59f57b..751c8b9f0 100644
>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> @@ -45,6 +45,7 @@ Required properties:
>>  	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
>>  	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
>>  	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
>> +	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
>>  	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
>>  	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
>>  	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
>> diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
>> index f4da52b..6753c87 100644
>> --- a/drivers/clk/sunxi/clk-simple-gates.c
>> +++ b/drivers/clk/sunxi/clk-simple-gates.c
>> @@ -130,6 +130,8 @@ CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
>>  	       sunxi_simple_gates_init);
>>  CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
>>  	       sunxi_simple_gates_init);
>> +CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
>> +	       sunxi_simple_gates_init);
>>  CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
>>  	       sunxi_simple_gates_init);
>>  CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",
>
>It seems that the other compatible strings are there for historical
>reasons. Why do you need a new one with such a specific name?
>
>It would have been more sensible to add a generic compatible string as
>"allwinner,apb-gates", letting the removal of the other strings for a
>later patch...

Seems like a good idea but is it possible to remove those binding in 
separate patch? I mean, wouldn't that, in theory, break ABI? Are such 
patches accepted?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard Feb. 5, 2016, 11:11 a.m. UTC | #3
Hi,

On Thu, Feb 04, 2016 at 03:47:52PM +0100, Jean-Francois Moine wrote:
> On Thu,  4 Feb 2016 00:33:46 +0100
> Krzysztof Adamski <k@japko.eu> wrote:
> 
> > This patch adds support for APB0 in H3. It seems to be compatible with
> > earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
> > etc).
> > 
> > Signed-off-by: Krzysztof Adamski <k@japko.eu>
> > ---
> >  Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
> >  drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> > index e59f57b..751c8b9f0 100644
> > --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> > +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> > @@ -45,6 +45,7 @@ Required properties:
> >  	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
> >  	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
> >  	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
> > +	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
> >  	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
> >  	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
> >  	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
> > diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
> > index f4da52b..6753c87 100644
> > --- a/drivers/clk/sunxi/clk-simple-gates.c
> > +++ b/drivers/clk/sunxi/clk-simple-gates.c
> > @@ -130,6 +130,8 @@ CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
> >  	       sunxi_simple_gates_init);
> >  CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
> >  	       sunxi_simple_gates_init);
> > +CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
> > +	       sunxi_simple_gates_init);
> >  CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
> >  	       sunxi_simple_gates_init);
> >  CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",
> 
> It seems that the other compatible strings are there for historical
> reasons. Why do you need a new one with such a specific name?
> 
> It would have been more sensible to add a generic compatible string as
> "allwinner,apb-gates", letting the removal of the other strings for a
> later patch...

Yeah, it's a good idea, and it's probably time that we move to that.

However, I'd like to keep per-soc and per-clocks compatibles in the
DT, in case we need to protect a clock in the future. That doesn't
prevent to have two compatibles thoughe, the specific and the generic.

Maxime
Maxime Ripard Feb. 5, 2016, 11:14 a.m. UTC | #4
Hi,

On Thu, Feb 04, 2016 at 09:56:02PM +0100, Krzysztof Adamski wrote:
> On Thu, Feb 04, 2016 at 03:47:52PM +0100, Jean-Francois Moine wrote:
> >On Thu,  4 Feb 2016 00:33:46 +0100
> >Krzysztof Adamski <k@japko.eu> wrote:
> >
> >>This patch adds support for APB0 in H3. It seems to be compatible with
> >>earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
> >>etc).
> >>
> >>Signed-off-by: Krzysztof Adamski <k@japko.eu>
> >>---
> >> Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
> >> drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
> >> 2 files changed, 3 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> >>index e59f57b..751c8b9f0 100644
> >>--- a/Documentation/devicetree/bindings/clock/sunxi.txt
> >>+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> >>@@ -45,6 +45,7 @@ Required properties:
> >> 	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
> >> 	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
> >> 	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
> >>+	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
> >> 	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
> >> 	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
> >> 	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
> >>diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
> >>index f4da52b..6753c87 100644
> >>--- a/drivers/clk/sunxi/clk-simple-gates.c
> >>+++ b/drivers/clk/sunxi/clk-simple-gates.c
> >>@@ -130,6 +130,8 @@ CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
> >> 	       sunxi_simple_gates_init);
> >> CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
> >> 	       sunxi_simple_gates_init);
> >>+CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
> >>+	       sunxi_simple_gates_init);
> >> CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
> >> 	       sunxi_simple_gates_init);
> >> CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",
> >
> >It seems that the other compatible strings are there for historical
> >reasons. Why do you need a new one with such a specific name?
> >
> >It would have been more sensible to add a generic compatible string as
> >"allwinner,apb-gates", letting the removal of the other strings for a
> >later patch...
> 
> Seems like a good idea but is it possible to remove those binding in
> separate patch?

Yeah, you can (and should) do that as a separate patch, that can be
part of the next iteration of this set.

> I mean, wouldn't that, in theory, break ABI? Are such
> patches accepted?

It would, but it doesn't matter.

Maxime
Krzysztof Adamski Feb. 5, 2016, 11:58 a.m. UTC | #5
On Fri, Feb 05, 2016 at 12:11:52PM +0100, Maxime Ripard wrote:
>Hi,
>
>On Thu, Feb 04, 2016 at 03:47:52PM +0100, Jean-Francois Moine wrote:
>> On Thu,  4 Feb 2016 00:33:46 +0100
>> Krzysztof Adamski <k@japko.eu> wrote:
>>
>> > This patch adds support for APB0 in H3. It seems to be compatible with
>> > earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
>> > etc).
>> >
>> > Signed-off-by: Krzysztof Adamski <k@japko.eu>
>> > ---
>> >  Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
>> >  drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
>> >  2 files changed, 3 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
>> > index e59f57b..751c8b9f0 100644
>> > --- a/Documentation/devicetree/bindings/clock/sunxi.txt
>> > +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
>> > @@ -45,6 +45,7 @@ Required properties:
>> >  	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
>> >  	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
>> >  	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
>> > +	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
>> >  	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
>> >  	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
>> >  	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
>> > diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
>> > index f4da52b..6753c87 100644
>> > --- a/drivers/clk/sunxi/clk-simple-gates.c
>> > +++ b/drivers/clk/sunxi/clk-simple-gates.c
>> > @@ -130,6 +130,8 @@ CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
>> >  	       sunxi_simple_gates_init);
>> >  CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
>> >  	       sunxi_simple_gates_init);
>> > +CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
>> > +	       sunxi_simple_gates_init);
>> >  CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
>> >  	       sunxi_simple_gates_init);
>> >  CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",
>>
>> It seems that the other compatible strings are there for historical
>> reasons. Why do you need a new one with such a specific name?
>>
>> It would have been more sensible to add a generic compatible string as
>> "allwinner,apb-gates", letting the removal of the other strings for a
>> later patch...
>
>Yeah, it's a good idea, and it's probably time that we move to that.
>
>However, I'd like to keep per-soc and per-clocks compatibles in the
>DT, in case we need to protect a clock in the future. That doesn't
>prevent to have two compatibles thoughe, the specific and the generic.
>

So now I'm not sure what you mean. You suggest that I should keep using 
specific (sun8i_h3_apb0) or change to generic (apb-gates) in my patch? 
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jean-Francois Moine Feb. 6, 2016, 10:26 a.m. UTC | #6
On Fri, 5 Feb 2016 12:11:52 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> However, I'd like to keep per-soc and per-clocks compatibles in the
> DT, in case we need to protect a clock in the future.

What do you mean by "protect a clock"?
Maxime Ripard Feb. 9, 2016, 5:10 p.m. UTC | #7
On Fri, Feb 05, 2016 at 12:58:37PM +0100, Krzysztof Adamski wrote:
> On Fri, Feb 05, 2016 at 12:11:52PM +0100, Maxime Ripard wrote:
> >Hi,
> >
> >On Thu, Feb 04, 2016 at 03:47:52PM +0100, Jean-Francois Moine wrote:
> >>On Thu,  4 Feb 2016 00:33:46 +0100
> >>Krzysztof Adamski <k@japko.eu> wrote:
> >>
> >>> This patch adds support for APB0 in H3. It seems to be compatible with
> >>> earlier SOCs. apb0 gates controls R_ block peripherals (R_PIO, R_IR,
> >>> etc).
> >>>
> >>> Signed-off-by: Krzysztof Adamski <k@japko.eu>
> >>> ---
> >>>  Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
> >>>  drivers/clk/sunxi/clk-simple-gates.c              | 2 ++
> >>>  2 files changed, 3 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> >>> index e59f57b..751c8b9f0 100644
> >>> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> >>> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> >>> @@ -45,6 +45,7 @@ Required properties:
> >>>  	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
> >>>  	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
> >>>  	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
> >>> +	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
> >>>  	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
> >>>  	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
> >>>  	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
> >>> diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
> >>> index f4da52b..6753c87 100644
> >>> --- a/drivers/clk/sunxi/clk-simple-gates.c
> >>> +++ b/drivers/clk/sunxi/clk-simple-gates.c
> >>> @@ -130,6 +130,8 @@ CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
> >>>  	       sunxi_simple_gates_init);
> >>>  CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
> >>>  	       sunxi_simple_gates_init);
> >>> +CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
> >>> +	       sunxi_simple_gates_init);
> >>>  CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
> >>>  	       sunxi_simple_gates_init);
> >>>  CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",
> >>
> >>It seems that the other compatible strings are there for historical
> >>reasons. Why do you need a new one with such a specific name?
> >>
> >>It would have been more sensible to add a generic compatible string as
> >>"allwinner,apb-gates", letting the removal of the other strings for a
> >>later patch...
> >
> >Yeah, it's a good idea, and it's probably time that we move to that.
> >
> >However, I'd like to keep per-soc and per-clocks compatibles in the
> >DT, in case we need to protect a clock in the future. That doesn't
> >prevent to have two compatibles thoughe, the specific and the generic.
> >
> 
> So now I'm not sure what you mean. You suggest that I should keep using
> specific (sun8i_h3_apb0) or change to generic (apb-gates) in my patch?

Both.

To have something like that:

compatible = "allwinner,sun8i-h3-apb0-gates-clk", "allwinner,sun4i-a10-gates-clk";

sun4i-a10-gates-clk being the generic compatible that we would use,
and we can always match against the h3 specific compatible if we need
to have a different behaviour.

Maxime
Maxime Ripard Feb. 9, 2016, 5:11 p.m. UTC | #8
On Sat, Feb 06, 2016 at 11:26:05AM +0100, Jean-Francois Moine wrote:
> On Fri, 5 Feb 2016 12:11:52 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > However, I'd like to keep per-soc and per-clocks compatibles in the
> > DT, in case we need to protect a clock in the future.
> 
> What do you mean by "protect a clock"?

Prevent it from shutting down.
Krzysztof Adamski Feb. 10, 2016, 7:17 a.m. UTC | #9
On Tue, Feb 09, 2016 at 06:10:40PM +0100, Maxime Ripard wrote:
>> >>It seems that the other compatible strings are there for historical
>> >>reasons. Why do you need a new one with such a specific name?
>> >>
>> >>It would have been more sensible to add a generic compatible string as
>> >>"allwinner,apb-gates", letting the removal of the other strings for a
>> >>later patch...
>> >
>> >Yeah, it's a good idea, and it's probably time that we move to that.
>> >
>> >However, I'd like to keep per-soc and per-clocks compatibles in the
>> >DT, in case we need to protect a clock in the future. That doesn't
>> >prevent to have two compatibles thoughe, the specific and the generic.
>> >
>>
>> So now I'm not sure what you mean. You suggest that I should keep using
>> specific (sun8i_h3_apb0) or change to generic (apb-gates) in my patch?
>
>Both.
>
>To have something like that:
>
>compatible = "allwinner,sun8i-h3-apb0-gates-clk", "allwinner,sun4i-a10-gates-clk";
>
>sun4i-a10-gates-clk being the generic compatible that we would use,
>and we can always match against the h3 specific compatible if we need
>to have a different behaviour.

This seems like a good idea to me but since this is new thing anyways 
(other sunxi SoCs don't do this right now) shouldn't we introduce other
more generic name for generic clock (like "allwinner,apb-gates" 
mentioned earlier, or maybe "allwinner,simple-apb-gates")?

Also, wouldn't this be cleaner if I use only specific compatible string 
right now and provide separate patch that adds generic compatible string 
to all current SoCs at once? The downside of this is that it's easier to 
get merge conflicts unless I wait for applying this patchset.

Best regards,
Krzysztof Adamski
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard Feb. 10, 2016, 3:06 p.m. UTC | #10
Hi,

On Wed, Feb 10, 2016 at 08:17:14AM +0100, Krzysztof Adamski wrote:
> On Tue, Feb 09, 2016 at 06:10:40PM +0100, Maxime Ripard wrote:
> >>>>It seems that the other compatible strings are there for historical
> >>>>reasons. Why do you need a new one with such a specific name?
> >>>>
> >>>>It would have been more sensible to add a generic compatible string as
> >>>>"allwinner,apb-gates", letting the removal of the other strings for a
> >>>>later patch...
> >>>
> >>>Yeah, it's a good idea, and it's probably time that we move to that.
> >>>
> >>>However, I'd like to keep per-soc and per-clocks compatibles in the
> >>>DT, in case we need to protect a clock in the future. That doesn't
> >>>prevent to have two compatibles thoughe, the specific and the generic.
> >>>
> >>
> >>So now I'm not sure what you mean. You suggest that I should keep using
> >>specific (sun8i_h3_apb0) or change to generic (apb-gates) in my patch?
> >
> >Both.
> >
> >To have something like that:
> >
> >compatible = "allwinner,sun8i-h3-apb0-gates-clk", "allwinner,sun4i-a10-gates-clk";
> >
> >sun4i-a10-gates-clk being the generic compatible that we would use,
> >and we can always match against the h3 specific compatible if we need
> >to have a different behaviour.
> 
> This seems like a good idea to me but since this is new thing anyways (other
> sunxi SoCs don't do this right now) shouldn't we introduce other
> more generic name for generic clock (like "allwinner,apb-gates" mentioned
> earlier, or maybe "allwinner,simple-apb-gates")?

This is not specific to the APB bus, and the earlier SoC that
introduced those kind of clocks was the A10, hence why I was
suggesting that compatible (since we generally try to use the SoC that
introduced that hardware block in the compatible name).

> Also, wouldn't this be cleaner if I use only specific compatible string
> right now and provide separate patch that adds generic compatible string to
> all current SoCs at once? The downside of this is that it's easier to get
> merge conflicts unless I wait for applying this patchset.

I'd prefer if you added the generic one and were using it in your
subsequent patches. We can always mass convert the existing drivers
later on.

Thanks!
Maxime
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index e59f57b..751c8b9f0 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -45,6 +45,7 @@  Required properties:
 	"allwinner,sun6i-a31-apb0-gates-clk" - for the APB0 gates on A31
 	"allwinner,sun7i-a20-apb0-gates-clk" - for the APB0 gates on A20
 	"allwinner,sun8i-a23-apb0-gates-clk" - for the APB0 gates on A23
+	"allwinner,sun8i-h3-apb0-gates-clk" - for the APB0 gates on H3
 	"allwinner,sun9i-a80-apb0-gates-clk" - for the APB0 gates on A80
 	"allwinner,sun4i-a10-apb1-clk" - for the APB1 clock
 	"allwinner,sun9i-a80-apb1-clk" - for the APB1 bus clock on A80
diff --git a/drivers/clk/sunxi/clk-simple-gates.c b/drivers/clk/sunxi/clk-simple-gates.c
index f4da52b..6753c87 100644
--- a/drivers/clk/sunxi/clk-simple-gates.c
+++ b/drivers/clk/sunxi/clk-simple-gates.c
@@ -130,6 +130,8 @@  CLK_OF_DECLARE(sun8i_a23_apb2, "allwinner,sun8i-a23-apb2-gates-clk",
 	       sunxi_simple_gates_init);
 CLK_OF_DECLARE(sun8i_a33_ahb1, "allwinner,sun8i-a33-ahb1-gates-clk",
 	       sunxi_simple_gates_init);
+CLK_OF_DECLARE(sun8i_h3_apb0, "allwinner,sun8i-h3-apb0-gates-clk",
+	       sunxi_simple_gates_init);
 CLK_OF_DECLARE(sun9i_a80_ahb0, "allwinner,sun9i-a80-ahb0-gates-clk",
 	       sunxi_simple_gates_init);
 CLK_OF_DECLARE(sun9i_a80_ahb1, "allwinner,sun9i-a80-ahb1-gates-clk",