Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
Hi Linus, On Mon, Sep 12, 2016 at 8:40 PM, Linus Walleij wrote: > On Thu, Sep 8, 2016 at 9:37 AM, Maxime Ripard > wrote: >> On Thu, Sep 08, 2016 at 12:46:14PM +0800, Chen-Yu Tsai wrote: > >>> Also, I think we are needlessly using pin groups, 1 pin per group. >>> Can pinconf/pinctrl work without them? Would there be any harm >>> converting the sunxi driver to work directly with pins? This would >>> make it match generic pinconf parsing, and make it easier to get >>> both working together. >> >> I think it comes from a requirement that you had to have groups at >> some point (I don't know if it's still the case), which is why we >> ended up with single-pin groups, because we can mux each pins entirely >> separately. >> >> If it's not required anymore, then yes, it makes total sense to remove >> it. > > The groups vs individual pins is an eternal debate that has > been going on since the inception of pinctrl. > > If you see it from the point of the programmer, you may just see > a register for each pin and they seem all independent. This is > why pinctrl-single exist, and that driver is for this purpose: one > register per pin, software-wise independent. > > HOWEVER it often turns out that while you can programmatically > and individually set pins to any function (and biasing etc), the > person designing the hardware was not thinking that you should > be able to do whatever you like, e.g. even if it is possible to > take two pins and use one of them for half an SPI bus and the > other for half an I2C bus, that doesn't mean that this is useful > or makes any kind of electronic sense, it just makes "software > sense". > > So for a deeper understanding, several SoCs (amongst them > my own and Qualcomm etc) define groups that are not really > about software restrictions for what you can do with the pins, but > about usecase and electronic restrictions for what can be done > with the pins. > > E.g. it makes *sense* to have a group for muxing I2C on two > pins, and not allow one of them to be muxed to I2C and the other > not, because it does not make electronic sense. > > One-group-per-pin groups is usually coming from a failure or > inability to identify these electronically sound and usecase > oriented pingroups. > > Some (like pinctrl-single) say they don't care, and wish to > see things as the world is just software and one register per > pin, removing those electric usecase restrictions, and only > keeping the muxing restrictions to e.g. the four different functions > that can be muxed on that pin, disregarding the bigger picture. > > I don't know about this driver or the pins it manages, > I seldom have time or brains to dive in and review things > deeply enough :( Thanks for the explanation. I suppose sunxi falls into the "don't care" group. We mainly enforce proper use cases through the DT pinmux settings. Of course this doesn't prevent the user from using weird settings out of tree, but then again what's preventing them from hacking the kernel anyway. Back to my original question: is it possible to drop the pin group support completely? Looking at struct pinctrl_ops the answer seems to be no. Regards ChenYu
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Thu, Sep 8, 2016 at 3:41 PM, Maxime Ripard
wrote:
> On Thu, Sep 08, 2016 at 12:32:48AM +0800, Chen-Yu Tsai wrote:
>> On Wed, Sep 7, 2016 at 10:53 PM, Maxime Ripard
>> wrote:
>> > From: Mylène Josserand
>> >
>> > The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
>> >
>> > Since it's not clear yet what we can factor out and merge with the A10s and
>> > A13 support, let's keep it out of the sun5i.dtsi include tree. We will
>> > figure out what can be shared when things settle down.
>> >
>> > Signed-off-by: Mylène Josserand
>> > Signed-off-by: Maxime Ripard
>> > ---
>> > arch/arm/boot/dts/ntc-gr8.dtsi | 1080
>> >
>> > 1 file changed, 1080 insertions(+)
>> > create mode 100644 arch/arm/boot/dts/ntc-gr8.dtsi
>> >
>> > diff --git a/arch/arm/boot/dts/ntc-gr8.dtsi
>> > b/arch/arm/boot/dts/ntc-gr8.dtsi
>> > new file mode 100644
>> > index ..d21cfa3f3c14
>> > --- /dev/null
>> > +++ b/arch/arm/boot/dts/ntc-gr8.dtsi
>> > @@ -0,0 +1,1080 @@
>>
>> [...]
>>
>> > + pll3x2: pll3x2_clk {
>> > + compatible = "fixed-factor-clock";
>>
>> I think you want "allwinner,sun4i-a10-pll3-2x-clk"?
>
> Indeed.
>
>> > + tcon_ch1_clk: clk@01c2012c {
>> > + #clock-cells = <0>;
>> > + compatible = "allwinner,sun4i-a10-tcon-ch1-clk";
>> > + reg = <0x01c2012c 0x4>;
>> > + clocks = <&pll3>, <&pll7>, <&pll3x2>, <&pll7x2>;
>> > + clock-output-names = "tcon-ch1-sclk";
>> > + };
>>
>> Nit: Is there a ve_clk we could add?
>
> I don't know. No one uses it, and the next item on my todo list is to
> move the sun5i SoCs to sunxi-ng, so it seems a bit useless to add that
> one.
Makes sense. Thanks!
ChenYu
>
>>
>> [...]
>>
>> > + pio: pinctrl@01c20800 {
>> > + compatible = "nextthing,gr8-pinctrl";
>> > + reg = <0x01c20800 0x400>;
>> > + interrupts = <28>;
>> > + clocks = <&apb0_gates 5>;
>> > + gpio-controller;
>> > + interrupt-controller;
>> > + #interrupt-cells = <3>;
>> > + #gpio-cells = <3>;
>> > +
>> > + i2c0_pins_a: i2c0@0 {
>> > + allwinner,pins = "PB0", "PB1";
>> > + allwinner,function = "i2c0";
>> > + allwinner,drive = ;
>> > + allwinner,pull = ;
>> > + };
>> > +
>> > + i2c1_pins_a: i2c1@0 {
>> > + allwinner,pins = "PB15", "PB16";
>> > + allwinner,function = "i2c1";
>> > + allwinner,drive = ;
>> > + allwinner,pull = ;
>> > + };
>> > +
>> > + i2c2_pins_a: i2c2@0 {
>> > + allwinner,pins = "PB17", "PB18";
>> > + allwinner,function = "i2c2";
>> > + allwinner,drive = ;
>> > + allwinner,pull = ;
>> > + };
>> > +
>> > + i2s0_pins_a: i2s0@0 {
>> > + allwinner,pins = "PB5", "PB6", "PB7",
>> > "PB8", "PB9";
>> > + allwinner,function = "i2s0";
>> > + allwinner,drive = ;
>> > + allwinner,pull = ;
>> > + };
>>
>> You may want to split out the MCLK pin. Some codecs don't need it, and the
>> pin can be allocated for other uses.
>
> ACK.
>
>>
>> > +
>> > + ir0_rx_pins_a: ir0@0 {
>> > + allwinner,pins = "PB4";
>> > + allwinner,function = "ir0";
>> > + allwinner,drive = ;
>> > + allwinner,pull = ;
>> > + };
>> > +
>> > + lcd_rgb666_pins: lcd_rgb666@0 {
>> > + allwinner,pins = "PD2", "PD3", "PD4",
>> > "PD5", "PD6", "PD7",
>> > +"PD10", "PD11", "PD12",
>> > "PD13", "PD14", "PD15",
>> > +"PD18", "PD19", "PD20",
>> > "PD21", "PD22", "PD23",
>> > +"PD24", "PD25", "PD26",
>> > "PD27";
>> > + allwinner,function = "lcd0";
>> > + allwinner,drive = ;
>> > + allwinner,pull = ;
>> > + };
>> > +
>> > + mmc0_pins_a: mmc0@0 {
>> > + allwinner,pins = "PF0", "PF1", "PF2",
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Mon, Sep 12, 2016 at 2:47 PM, Laurent Pinchart wrote: > On Monday 12 Sep 2016 14:40:15 Linus Walleij wrote: >> HOWEVER it often turns out that while you can programmatically >> and individually set pins to any function (and biasing etc), the >> person designing the hardware was not thinking that you should >> be able to do whatever you like, e.g. even if it is possible to >> take two pins and use one of them for half an SPI bus and the >> other for half an I2C bus, that doesn't mean that this is useful >> or makes any kind of electronic sense, it just makes "software >> sense". (...) > I'd argue that you would find out about lots of clever/insane use cases that > don't fit this model if you looked at all the hardware available out there, > especially non-phone devices. Your SPI example is a good one, I've seen SPI > being used in unidirectional mode only, with only MISO or MOSI mattering. In > that case the other pin could be used as a GPIO for a totally unrelated > purpose when the design is short on GPIOs or when GPIOs have been allocated > without any knowledge of the Linux pinctrl subsystem. That is true sometimes. It is a tradeoff, I can also imagine actually driving an I2C bus just to use the SCL line as a clock for something, constantly feeding nonsense data through the I2C block and ignoring SDA and reusing that pin as GPIO. (And a lot of other theoretical usecases.) Some pin controller hardware helpully only let you select groups and makes such hacks impossible. Also I guess the target audience of the SoC will affect the hackishness of the usecases, and affect what they might attempt to shoehorn into the design. So model on whatever makes most sense, is usually how I think about it. Or as the IETF says "rough consensus and running code". I guess it is a bit of grayzone, and that is why both solutions coexist. > Looking at the sh-pfc driver, I wish the hardware had followed the pinctrl- > single model. sh-pfc is a good example of how bloated a pinctrl driver can > become when there is no choice but model all the relationships betweens pins > and functions in C code. It might be true, there are so many variables to the equation that I cannot tell. Debuggability and readability of code and device trees and different groups of people reading code vs device trees is another factor. Scaringly, what is best for me as subsystem maintainer (that all drivers look identical) is not always best for the users. Yours, Linus Walleij
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
Hi Linus, On Monday 12 Sep 2016 14:40:15 Linus Walleij wrote: > On Thu, Sep 8, 2016 at 9:37 AM, Maxime Ripard wrote: > > On Thu, Sep 08, 2016 at 12:46:14PM +0800, Chen-Yu Tsai wrote: > >> Also, I think we are needlessly using pin groups, 1 pin per group. > >> Can pinconf/pinctrl work without them? Would there be any harm > >> converting the sunxi driver to work directly with pins? This would > >> make it match generic pinconf parsing, and make it easier to get > >> both working together. > > > > I think it comes from a requirement that you had to have groups at > > some point (I don't know if it's still the case), which is why we > > ended up with single-pin groups, because we can mux each pins entirely > > separately. > > > > If it's not required anymore, then yes, it makes total sense to remove > > it. > > The groups vs individual pins is an eternal debate that has > been going on since the inception of pinctrl. > > If you see it from the point of the programmer, you may just see > a register for each pin and they seem all independent. This is > why pinctrl-single exist, and that driver is for this purpose: one > register per pin, software-wise independent. > > HOWEVER it often turns out that while you can programmatically > and individually set pins to any function (and biasing etc), the > person designing the hardware was not thinking that you should > be able to do whatever you like, e.g. even if it is possible to > take two pins and use one of them for half an SPI bus and the > other for half an I2C bus, that doesn't mean that this is useful > or makes any kind of electronic sense, it just makes "software > sense". > > So for a deeper understanding, several SoCs (amongst them > my own and Qualcomm etc) define groups that are not really > about software restrictions for what you can do with the pins, but > about usecase and electronic restrictions for what can be done > with the pins. > > E.g. it makes *sense* to have a group for muxing I2C on two > pins, and not allow one of them to be muxed to I2C and the other > not, because it does not make electronic sense. > > One-group-per-pin groups is usually coming from a failure or > inability to identify these electronically sound and usecase > oriented pingroups. I'd argue that you would find out about lots of clever/insane use cases that don't fit this model if you looked at all the hardware available out there, especially non-phone devices. Your SPI example is a good one, I've seen SPI being used in unidirectional mode only, with only MISO or MOSI mattering. In that case the other pin could be used as a GPIO for a totally unrelated purpose when the design is short on GPIOs or when GPIOs have been allocated without any knowledge of the Linux pinctrl subsystem. Looking at the sh-pfc driver, I wish the hardware had followed the pinctrl- single model. sh-pfc is a good example of how bloated a pinctrl driver can become when there is no choice but model all the relationships betweens pins and functions in C code. > Some (like pinctrl-single) say they don't care, and wish to > see things as the world is just software and one register per > pin, removing those electric usecase restrictions, and only > keeping the muxing restrictions to e.g. the four different functions > that can be muxed on that pin, disregarding the bigger picture. > > I don't know about this driver or the pins it manages, > I seldom have time or brains to dive in and review things > deeply enough :( -- Regards, Laurent Pinchart
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Thu, Sep 8, 2016 at 9:37 AM, Maxime Ripard wrote: > On Thu, Sep 08, 2016 at 12:46:14PM +0800, Chen-Yu Tsai wrote: >> Also, I think we are needlessly using pin groups, 1 pin per group. >> Can pinconf/pinctrl work without them? Would there be any harm >> converting the sunxi driver to work directly with pins? This would >> make it match generic pinconf parsing, and make it easier to get >> both working together. > > I think it comes from a requirement that you had to have groups at > some point (I don't know if it's still the case), which is why we > ended up with single-pin groups, because we can mux each pins entirely > separately. > > If it's not required anymore, then yes, it makes total sense to remove > it. The groups vs individual pins is an eternal debate that has been going on since the inception of pinctrl. If you see it from the point of the programmer, you may just see a register for each pin and they seem all independent. This is why pinctrl-single exist, and that driver is for this purpose: one register per pin, software-wise independent. HOWEVER it often turns out that while you can programmatically and individually set pins to any function (and biasing etc), the person designing the hardware was not thinking that you should be able to do whatever you like, e.g. even if it is possible to take two pins and use one of them for half an SPI bus and the other for half an I2C bus, that doesn't mean that this is useful or makes any kind of electronic sense, it just makes "software sense". So for a deeper understanding, several SoCs (amongst them my own and Qualcomm etc) define groups that are not really about software restrictions for what you can do with the pins, but about usecase and electronic restrictions for what can be done with the pins. E.g. it makes *sense* to have a group for muxing I2C on two pins, and not allow one of them to be muxed to I2C and the other not, because it does not make electronic sense. One-group-per-pin groups is usually coming from a failure or inability to identify these electronically sound and usecase oriented pingroups. Some (like pinctrl-single) say they don't care, and wish to see things as the world is just software and one register per pin, removing those electric usecase restrictions, and only keeping the muxing restrictions to e.g. the four different functions that can be muxed on that pin, disregarding the bigger picture. I don't know about this driver or the pins it manages, I seldom have time or brains to dive in and review things deeply enough :( Yours, Linus Walleij
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Thu, Sep 08, 2016 at 12:32:48AM +0800, Chen-Yu Tsai wrote:
> On Wed, Sep 7, 2016 at 10:53 PM, Maxime Ripard
> wrote:
> > From: Mylène Josserand
> >
> > The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
> >
> > Since it's not clear yet what we can factor out and merge with the A10s and
> > A13 support, let's keep it out of the sun5i.dtsi include tree. We will
> > figure out what can be shared when things settle down.
> >
> > Signed-off-by: Mylène Josserand
> > Signed-off-by: Maxime Ripard
> > ---
> > arch/arm/boot/dts/ntc-gr8.dtsi | 1080
> >
> > 1 file changed, 1080 insertions(+)
> > create mode 100644 arch/arm/boot/dts/ntc-gr8.dtsi
> >
> > diff --git a/arch/arm/boot/dts/ntc-gr8.dtsi b/arch/arm/boot/dts/ntc-gr8.dtsi
> > new file mode 100644
> > index ..d21cfa3f3c14
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/ntc-gr8.dtsi
> > @@ -0,0 +1,1080 @@
>
> [...]
>
> > + pll3x2: pll3x2_clk {
> > + compatible = "fixed-factor-clock";
>
> I think you want "allwinner,sun4i-a10-pll3-2x-clk"?
Indeed.
> > + tcon_ch1_clk: clk@01c2012c {
> > + #clock-cells = <0>;
> > + compatible = "allwinner,sun4i-a10-tcon-ch1-clk";
> > + reg = <0x01c2012c 0x4>;
> > + clocks = <&pll3>, <&pll7>, <&pll3x2>, <&pll7x2>;
> > + clock-output-names = "tcon-ch1-sclk";
> > + };
>
> Nit: Is there a ve_clk we could add?
I don't know. No one uses it, and the next item on my todo list is to
move the sun5i SoCs to sunxi-ng, so it seems a bit useless to add that
one.
>
> [...]
>
> > + pio: pinctrl@01c20800 {
> > + compatible = "nextthing,gr8-pinctrl";
> > + reg = <0x01c20800 0x400>;
> > + interrupts = <28>;
> > + clocks = <&apb0_gates 5>;
> > + gpio-controller;
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > + #gpio-cells = <3>;
> > +
> > + i2c0_pins_a: i2c0@0 {
> > + allwinner,pins = "PB0", "PB1";
> > + allwinner,function = "i2c0";
> > + allwinner,drive = ;
> > + allwinner,pull = ;
> > + };
> > +
> > + i2c1_pins_a: i2c1@0 {
> > + allwinner,pins = "PB15", "PB16";
> > + allwinner,function = "i2c1";
> > + allwinner,drive = ;
> > + allwinner,pull = ;
> > + };
> > +
> > + i2c2_pins_a: i2c2@0 {
> > + allwinner,pins = "PB17", "PB18";
> > + allwinner,function = "i2c2";
> > + allwinner,drive = ;
> > + allwinner,pull = ;
> > + };
> > +
> > + i2s0_pins_a: i2s0@0 {
> > + allwinner,pins = "PB5", "PB6", "PB7",
> > "PB8", "PB9";
> > + allwinner,function = "i2s0";
> > + allwinner,drive = ;
> > + allwinner,pull = ;
> > + };
>
> You may want to split out the MCLK pin. Some codecs don't need it, and the
> pin can be allocated for other uses.
ACK.
>
> > +
> > + ir0_rx_pins_a: ir0@0 {
> > + allwinner,pins = "PB4";
> > + allwinner,function = "ir0";
> > + allwinner,drive = ;
> > + allwinner,pull = ;
> > + };
> > +
> > + lcd_rgb666_pins: lcd_rgb666@0 {
> > + allwinner,pins = "PD2", "PD3", "PD4",
> > "PD5", "PD6", "PD7",
> > +"PD10", "PD11", "PD12",
> > "PD13", "PD14", "PD15",
> > +"PD18", "PD19", "PD20",
> > "PD21", "PD22", "PD23",
> > +"PD24", "PD25", "PD26",
> > "PD27";
> > + allwinner,function = "lcd0";
> > + allwinner,drive = ;
> > + allwinner,pull = ;
> > + };
> > +
> > + mmc0_pins_a: mmc0@0 {
> > + allwinner,pins = "PF0", "PF1", "PF2", "PF3",
> > +"PF4", "PF5";
> > + allwinner,function = "mmc0";
> > + allwinner,drive = ;
> > +
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Thu, Sep 08, 2016 at 12:46:14PM +0800, Chen-Yu Tsai wrote:
> > I mean it like supporting these in *addition* to the custom ones, so there
> > can
> > be a smooth phase-over.
> >
> > Check for example Laurent's commit for SH-PFC:
> > commit 16ccaf5bb5a52372bfebd3dfbb79dd810ad49c09
> > "pinctrl: sh-pfc: Accept standard function, pins and groups properties"
> > It's awesome, and since, they have improved the looks of Renesas
> > DTS files a lot.
> >
> > It could look a bit like this nice thing from
> > lpc4337-ciaa.dts:
> >
> > &pinctrl {
> > enet_rmii_pins: enet-rmii-pins {
> > enet_rmii_rxd_cfg {
> > pins = "p1_15", "p0_0";
> > function = "enet";
> > slew-rate = <1>;
> > bias-disable;
> > input-enable;
> > input-schmitt-disable;
> > };
> >
> > enet_rmii_txd_cfg {
> > pins = "p1_18", "p1_20";
> > function = "enet";
> > slew-rate = <1>;
> > bias-disable;
> > input-enable;
> > input-schmitt-disable;
> > };
> > (etc)
>
> This looks nice.
Indeed.
> I've slightly looked at the generic pinconf stuff. I think we
> should be able to support them, though the sunxi pinctrl driver
> currently doesn't work well with it though. For example, it doesn't
> declare ".is_generic = true", it doesn't filter unsupported pinconf
> parameters, and it doesn't reply to queries correctly. I will fix
> these bits.
>
> Also, I think we are needlessly using pin groups, 1 pin per group.
> Can pinconf/pinctrl work without them? Would there be any harm
> converting the sunxi driver to work directly with pins? This would
> make it match generic pinconf parsing, and make it easier to get
> both working together.
I think it comes from a requirement that you had to have groups at
some point (I don't know if it's still the case), which is why we
ended up with single-pin groups, because we can mux each pins entirely
separately.
If it's not required anymore, then yes, it makes total sense to remove
it.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
signature.asc
Description: PGP signature
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Thu, Sep 8, 2016 at 3:37 AM, Linus Walleij wrote:
> On Wed, Sep 7, 2016 at 4:53 PM, Maxime Ripard
> wrote:
>
>> From: Mylène Josserand
>>
>> The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
>>
>> Since it's not clear yet what we can factor out and merge with the A10s and
>> A13 support, let's keep it out of the sun5i.dtsi include tree. We will
>> figure out what can be shared when things settle down.
>>
>> Signed-off-by: Mylène Josserand
>> Signed-off-by: Maxime Ripard
>
> Acked-by: Linus Walleij
>
> I was just thinking:
>
>> + i2c0_pins_a: i2c0@0 {
>> + allwinner,pins = "PB0", "PB1";
>> + allwinner,function = "i2c0";
>> + allwinner,drive = ;
>> + allwinner,pull = ;
>> + };
>
> It would be *NICE* if the sunxi driver would start to support the new standard
> bindings for this stuff, see
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>
> So you could just use pins, function and the drive-strength and
> bias-disable in this case.
>
> Since I know the AllWinner support is a community project I have much higher
> tolerance with this legacy binding sticking around for the new generation of
> SoCs but still, if you find time.
>
> I mean it like supporting these in *addition* to the custom ones, so there can
> be a smooth phase-over.
>
> Check for example Laurent's commit for SH-PFC:
> commit 16ccaf5bb5a52372bfebd3dfbb79dd810ad49c09
> "pinctrl: sh-pfc: Accept standard function, pins and groups properties"
> It's awesome, and since, they have improved the looks of Renesas
> DTS files a lot.
>
> It could look a bit like this nice thing from
> lpc4337-ciaa.dts:
>
> &pinctrl {
> enet_rmii_pins: enet-rmii-pins {
> enet_rmii_rxd_cfg {
> pins = "p1_15", "p0_0";
> function = "enet";
> slew-rate = <1>;
> bias-disable;
> input-enable;
> input-schmitt-disable;
> };
>
> enet_rmii_txd_cfg {
> pins = "p1_18", "p1_20";
> function = "enet";
> slew-rate = <1>;
> bias-disable;
> input-enable;
> input-schmitt-disable;
> };
> (etc)
This looks nice. I've slightly looked at the generic pinconf stuff.
I think we should be able to support them, though the sunxi pinctrl
driver currently doesn't work well with it though. For example,
it doesn't declare ".is_generic = true", it doesn't filter
unsupported pinconf parameters, and it doesn't reply to queries
correctly. I will fix these bits.
Also, I think we are needlessly using pin groups, 1 pin per group.
Can pinconf/pinctrl work without them? Would there be any harm
converting the sunxi driver to work directly with pins? This would
make it match generic pinconf parsing, and make it easier to get
both working together.
Regards
ChenYu
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Wed, Sep 7, 2016 at 4:53 PM, Maxime Ripard
wrote:
> From: Mylène Josserand
>
> The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
>
> Since it's not clear yet what we can factor out and merge with the A10s and
> A13 support, let's keep it out of the sun5i.dtsi include tree. We will
> figure out what can be shared when things settle down.
>
> Signed-off-by: Mylène Josserand
> Signed-off-by: Maxime Ripard
Acked-by: Linus Walleij
I was just thinking:
> + i2c0_pins_a: i2c0@0 {
> + allwinner,pins = "PB0", "PB1";
> + allwinner,function = "i2c0";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
It would be *NICE* if the sunxi driver would start to support the new standard
bindings for this stuff, see
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
So you could just use pins, function and the drive-strength and
bias-disable in this case.
Since I know the AllWinner support is a community project I have much higher
tolerance with this legacy binding sticking around for the new generation of
SoCs but still, if you find time.
I mean it like supporting these in *addition* to the custom ones, so there can
be a smooth phase-over.
Check for example Laurent's commit for SH-PFC:
commit 16ccaf5bb5a52372bfebd3dfbb79dd810ad49c09
"pinctrl: sh-pfc: Accept standard function, pins and groups properties"
It's awesome, and since, they have improved the looks of Renesas
DTS files a lot.
It could look a bit like this nice thing from
lpc4337-ciaa.dts:
&pinctrl {
enet_rmii_pins: enet-rmii-pins {
enet_rmii_rxd_cfg {
pins = "p1_15", "p0_0";
function = "enet";
slew-rate = <1>;
bias-disable;
input-enable;
input-schmitt-disable;
};
enet_rmii_txd_cfg {
pins = "p1_18", "p1_20";
function = "enet";
slew-rate = <1>;
bias-disable;
input-enable;
input-schmitt-disable;
};
(etc)
Yours,
Linus Walleij
Re: [PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
On Wed, Sep 7, 2016 at 10:53 PM, Maxime Ripard
wrote:
> From: Mylène Josserand
>
> The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
>
> Since it's not clear yet what we can factor out and merge with the A10s and
> A13 support, let's keep it out of the sun5i.dtsi include tree. We will
> figure out what can be shared when things settle down.
>
> Signed-off-by: Mylène Josserand
> Signed-off-by: Maxime Ripard
> ---
> arch/arm/boot/dts/ntc-gr8.dtsi | 1080
>
> 1 file changed, 1080 insertions(+)
> create mode 100644 arch/arm/boot/dts/ntc-gr8.dtsi
>
> diff --git a/arch/arm/boot/dts/ntc-gr8.dtsi b/arch/arm/boot/dts/ntc-gr8.dtsi
> new file mode 100644
> index ..d21cfa3f3c14
> --- /dev/null
> +++ b/arch/arm/boot/dts/ntc-gr8.dtsi
> @@ -0,0 +1,1080 @@
[...]
> + pll3x2: pll3x2_clk {
> + compatible = "fixed-factor-clock";
I think you want "allwinner,sun4i-a10-pll3-2x-clk"?
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <2>;
> + clocks = <&pll3>;
> + clock-output-names = "pll3-2x";
> + };
[...]
> + pll7x2: pll7x2_clk {
> + compatible = "fixed-factor-clock";
Same here.
> + #clock-cells = <0>;
> + clock-div = <1>;
> + clock-mult = <2>;
> + clocks = <&pll7>;
> + clock-output-names = "pll7-2x";
> + };
> +
[...]
> + ahb_gates: clk@01c20060 {
> + #clock-cells = <1>;
> + compatible = "allwinner,sun5i-a13-ahb-gates-clk";
> + reg = <0x01c20060 0x8>;
> + clocks = <&ahb>;
> + clock-indices = <0>, <1>,
> + <2>, <5>, <6>,
> + <7>, <8>, <9>,
> + <10>, <13>,
> + <14>, <20>,
> + <21>, <22>,
> + <28>, <32>, <34>,
> + <36>, <40>, <44>,
> + <46>, <51>,
> + <52>;
> + clock-output-names = "ahb_usbotg", "ahb_ehci",
> +"ahb_ohci", "ahb_ss", "ahb_dma",
> +"ahb_bist", "ahb_mmc0",
> "ahb_mmc1",
> +"ahb_mmc2", "ahb_nand",
> +"ahb_sdram", "ahb_spi0",
> +"ahb_spi1", "ahb_spi2",
> +"ahb_stimer", "ahb_ve",
> "ahb_tve",
"ahb_hstimer"?
> +"ahb_lcd", "ahb_csi",
> "ahb_de_be",
> +"ahb_de_fe", "ahb_iep",
> +"ahb_mali400";
> + };
[...]
> + tcon_ch1_clk: clk@01c2012c {
> + #clock-cells = <0>;
> + compatible = "allwinner,sun4i-a10-tcon-ch1-clk";
> + reg = <0x01c2012c 0x4>;
> + clocks = <&pll3>, <&pll7>, <&pll3x2>, <&pll7x2>;
> + clock-output-names = "tcon-ch1-sclk";
> + };
Nit: Is there a ve_clk we could add?
[...]
> + pio: pinctrl@01c20800 {
> + compatible = "nextthing,gr8-pinctrl";
> + reg = <0x01c20800 0x400>;
> + interrupts = <28>;
> + clocks = <&apb0_gates 5>;
> + gpio-controller;
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + #gpio-cells = <3>;
> +
> + i2c0_pins_a: i2c0@0 {
> + allwinner,pins = "PB0", "PB1";
> + allwinner,function = "i2c0";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + i2c1_pins_a: i2c1@0 {
> + allwinner,pins = "PB15", "PB16";
> + allwinner,function = "i2c1";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + i2c2_pins_a: i2c2@0 {
> + allwinner,pins = "PB17", "PB18";
> + allwinner,function = "i2c2";
> +
[PATCH v2 3/4] ARM: dts: Add NextThing GR8 dtsi
From: Mylène Josserand
The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
Since it's not clear yet what we can factor out and merge with the A10s and
A13 support, let's keep it out of the sun5i.dtsi include tree. We will
figure out what can be shared when things settle down.
Signed-off-by: Mylène Josserand
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/ntc-gr8.dtsi | 1080
1 file changed, 1080 insertions(+)
create mode 100644 arch/arm/boot/dts/ntc-gr8.dtsi
diff --git a/arch/arm/boot/dts/ntc-gr8.dtsi b/arch/arm/boot/dts/ntc-gr8.dtsi
new file mode 100644
index ..d21cfa3f3c14
--- /dev/null
+++ b/arch/arm/boot/dts/ntc-gr8.dtsi
@@ -0,0 +1,1080 @@
+/*
+ * Copyright 2016 Mylène Josserand
+ *
+ * Mylène Josserand
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include "skeleton.dtsi"
+
+#include
+#include
+#include
+
+/ {
+ interrupt-parent = <&intc>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a8";
+ reg = <0x0>;
+ clocks = <&cpu>;
+ };
+ };
+
+ clocks {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ /*
+* This is a dummy clock, to be used as placeholder on
+* other mux clocks when a specific parent clock is not
+* yet implemented. It should be dropped when the driver
+* is complete.
+*/
+ dummy: dummy {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <0>;
+ };
+
+ osc24M: clk@01c20050 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-a10-osc-clk";
+ reg = <0x01c20050 0x4>;
+ clock-frequency = <2400>;
+ clock-output-names = "osc24M";
+ };
+
+ osc3M: osc3M_clk {
+ compatible = "fixed-factor-clock";
+ #clock-cells = <0>;
+ clock-div = <8>;
+ clock-mult = <1>;
+ clocks = <&osc24M>;
+ clock-output-names = "osc3M";
+ };
+
+ osc32k: clk@0 {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <32768>;
+ clock-output-names = "osc32k";
+ };
+
+ pll1: clk@01c2 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-a10-pll1-clk";
+ reg = <0x01c2 0x4>;
+ clocks = <&osc24M>;
+
