[PATCH 0/2] pinctrl: rza1: add support for RZ/A1L

[PATCH 0/2] pinctrl: rza1: add support for RZ/A1L

[Chris Brandt]

The RZ/A series has 3 main device: RZ/A1L, RZ/A1M, RZ/A1H.

In terms of pinctrl, RZ/A1H and RZ/A1M are exactly the same.

Aspects like the number of ports and the location where peripherals are
brought out differ between the RZ/A1H and RZ/A1L.

Basically this series adds a new set of tables for RZ/A1L and a new
compatible "renesas,r7s72102-ports"

This was tested on a Renesas Stream it board which contains an RZ/A1L and
external SDRAM.


Chris Brandt (2):
  pinctrl: rza1: add support for RZ/A1L
  dt-bindings: pinctrl: add support for RZ/A1L

 .../bindings/pinctrl/renesas,rza1-pinctrl.txt      |   3 +-
 drivers/pinctrl/pinctrl-rza1.c                     | 134 +++++++++++++++++++++
 2 files changed, 136 insertions(+), 1 deletion(-)

-- 
2.14.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] pinctrl: rza1: add support for RZ/A1L

[Chris Brandt]

Aspects like the number of ports and the location where peripherals are
brought out differ between the RZ/A1H and RZ/A1L.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 drivers/pinctrl/pinctrl-rza1.c | 134 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 134 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c
index 04d058706b80..717c0f4449a0 100644
--- a/drivers/pinctrl/pinctrl-rza1.c
+++ b/drivers/pinctrl/pinctrl-rza1.c
@@ -302,6 +302,134 @@ static const struct rza1_pinmux_conf rza1h_pmx_conf = {
 	.swio_entries	= rza1h_swio_entries,
 };
 
+/* ----------------------------------------------------------------------------
+ * RZ/A1L (r7s72102) pinmux flags
+ */
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p1[] = {
+	{ .pin = 0, .func = 1 },
+	{ .pin = 1, .func = 1 },
+	{ .pin = 2, .func = 1 },
+	{ .pin = 3, .func = 1 },
+	{ .pin = 4, .func = 1 },
+	{ .pin = 5, .func = 1 },
+	{ .pin = 6, .func = 1 },
+	{ .pin = 7, .func = 1 },
+};
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p3[] = {
+	{ .pin = 0, .func = 2 },
+	{ .pin = 1, .func = 2 },
+	{ .pin = 2, .func = 2 },
+	{ .pin = 4, .func = 2 },
+	{ .pin = 5, .func = 2 },
+	{ .pin = 10, .func = 2 },
+	{ .pin = 11, .func = 2 },
+	{ .pin = 12, .func = 2 },
+	{ .pin = 13, .func = 2 },
+};
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p4[] = {
+	{ .pin = 1, .func = 4 },
+	{ .pin = 2, .func = 2 },
+	{ .pin = 3, .func = 2 },
+	{ .pin = 6, .func = 2 },
+	{ .pin = 7, .func = 2 },
+};
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p5[] = {
+	{ .pin = 0, .func = 1 },
+	{ .pin = 1, .func = 1 },
+	{ .pin = 2, .func = 1 },
+	{ .pin = 3, .func = 1 },
+	{ .pin = 4, .func = 1 },
+	{ .pin = 5, .func = 1 },
+	{ .pin = 6, .func = 1 },
+	{ .pin = 7, .func = 1 },
+	{ .pin = 8, .func = 1 },
+	{ .pin = 9, .func = 1 },
+	{ .pin = 10, .func = 1 },
+	{ .pin = 11, .func = 1 },
+	{ .pin = 12, .func = 1 },
+	{ .pin = 13, .func = 1 },
+	{ .pin = 14, .func = 1 },
+	{ .pin = 15, .func = 1 },
+	{ .pin = 0, .func = 2 },
+	{ .pin = 1, .func = 2 },
+	{ .pin = 2, .func = 2 },
+	{ .pin = 3, .func = 2 },
+};
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p6[] = {
+	{ .pin = 0, .func = 1 },
+	{ .pin = 1, .func = 1 },
+	{ .pin = 2, .func = 1 },
+	{ .pin = 3, .func = 1 },
+	{ .pin = 4, .func = 1 },
+	{ .pin = 5, .func = 1 },
+	{ .pin = 6, .func = 1 },
+	{ .pin = 7, .func = 1 },
+	{ .pin = 8, .func = 1 },
+	{ .pin = 9, .func = 1 },
+	{ .pin = 10, .func = 1 },
+	{ .pin = 11, .func = 1 },
+	{ .pin = 12, .func = 1 },
+	{ .pin = 13, .func = 1 },
+	{ .pin = 14, .func = 1 },
+	{ .pin = 15, .func = 1 },
+};
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p7[] = {
+	{ .pin = 2, .func = 2 },
+	{ .pin = 3, .func = 2 },
+	{ .pin = 5, .func = 2 },
+	{ .pin = 6, .func = 2 },
+	{ .pin = 7, .func = 2 },
+	{ .pin = 2, .func = 3 },
+	{ .pin = 3, .func = 3 },
+	{ .pin = 5, .func = 3 },
+	{ .pin = 6, .func = 3 },
+	{ .pin = 7, .func = 3 },
+};
+
+static const struct rza1_bidir_pin rza1l_bidir_pins_p9[] = {
+	{ .pin = 1, .func = 2 },
+	{ .pin = 0, .func = 3 },
+	{ .pin = 1, .func = 3 },
+	{ .pin = 3, .func = 3 },
+	{ .pin = 4, .func = 3 },
+	{ .pin = 5, .func = 3 },
+};
+
+static const struct rza1_swio_pin rza1l_swio_pins[] = {
+	{ .port = 2, .pin = 8, .func = 2, .input = 0 },
+	{ .port = 5, .pin = 6, .func = 3, .input = 0 },
+	{ .port = 6, .pin = 6, .func = 3, .input = 0 },
+	{ .port = 6, .pin = 10, .func = 3, .input = 0 },
+	{ .port = 7, .pin = 10, .func = 2, .input = 0 },
+	{ .port = 8, .pin = 2, .func = 3, .input = 0 },
+};
+
+static const struct rza1_bidir_entry rza1l_bidir_entries[RZA1_NPORTS] = {
+	[1] = { ARRAY_SIZE(rza1l_bidir_pins_p1), rza1l_bidir_pins_p1 },
+	[3] = { ARRAY_SIZE(rza1l_bidir_pins_p3), rza1l_bidir_pins_p3 },
+	[4] = { ARRAY_SIZE(rza1l_bidir_pins_p4), rza1l_bidir_pins_p4 },
+	[5] = { ARRAY_SIZE(rza1l_bidir_pins_p4), rza1l_bidir_pins_p5 },
+	[6] = { ARRAY_SIZE(rza1l_bidir_pins_p6), rza1l_bidir_pins_p6 },
+	[7] = { ARRAY_SIZE(rza1l_bidir_pins_p7), rza1l_bidir_pins_p7 },
+	[9] = { ARRAY_SIZE(rza1l_bidir_pins_p9), rza1l_bidir_pins_p9 },
+};
+
+static const struct rza1_swio_entry rza1l_swio_entries[] = {
+	[0] = { ARRAY_SIZE(rza1h_swio_pins), rza1h_swio_pins },
+};
+
+/* RZ/A1L (r7s72102x) pinmux flags table */
+static const struct rza1_pinmux_conf rza1l_pmx_conf = {
+	.bidir_entries	= rza1l_bidir_entries,
+	.swio_entries	= rza1l_swio_entries,
+};
+
 /* ----------------------------------------------------------------------------
  * RZ/A1 types
  */
@@ -1283,9 +1411,15 @@ static int rza1_pinctrl_probe(struct platform_device *pdev)
 
 static const struct of_device_id rza1_pinctrl_of_match[] = {
 	{
+		/* RZ/A1H, RZ/A1M */
 		.compatible	= "renesas,r7s72100-ports",
 		.data		= &rza1h_pmx_conf,
 	},
+	{
+		/* RZ/A1L */
+		.compatible	= "renesas,r7s72102-ports",
+		.data		= &rza1l_pmx_conf,
+	},
 	{ }
 };
 
-- 
2.14.1

Re: [PATCH 1/2] pinctrl: rza1: add support for RZ/A1L

[jacopo mondi]

Hi Chris,
   thanks for the patches

On Tue, Oct 03, 2017 at 02:33:33AM -0500, Chris Brandt wrote:
> Aspects like the number of ports and the location where peripherals are
> brought out differ between the RZ/A1H and RZ/A1L.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  drivers/pinctrl/pinctrl-rza1.c | 134 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 134 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c
> index 04d058706b80..717c0f4449a0 100644
> --- a/drivers/pinctrl/pinctrl-rza1.c
> +++ b/drivers/pinctrl/pinctrl-rza1.c
> @@ -302,6 +302,134 @@ static const struct rza1_pinmux_conf rza1h_pmx_conf = {
>  	.swio_entries	= rza1h_swio_entries,
>  };
>
> +/* ----------------------------------------------------------------------------
> + * RZ/A1L (r7s72102) pinmux flags
> + */

Do you have other authoritative sources for pin's bidir settings?
According to the publicly available manual, there are other pins that
needs bidir not listed here.
As the same happened with RZ/A1H I'm not too concerned, but I'm
listing some of there here nonetheless

> +
> +static const struct rza1_bidir_pin rza1l_bidir_pins_p1[] = {
> +	{ .pin = 0, .func = 1 },
> +	{ .pin = 1, .func = 1 },
> +	{ .pin = 2, .func = 1 },
> +	{ .pin = 3, .func = 1 },
> +	{ .pin = 4, .func = 1 },
> +	{ .pin = 5, .func = 1 },
> +	{ .pin = 6, .func = 1 },
> +	{ .pin = 7, .func = 1 },
> +};
> +

P2 has several TIOC pins in mode 4 and SSI in mode 2

> +static const struct rza1_bidir_pin rza1l_bidir_pins_p3[] = {
> +	{ .pin = 0, .func = 2 },
> +	{ .pin = 1, .func = 2 },
> +	{ .pin = 2, .func = 2 },
> +	{ .pin = 4, .func = 2 },
> +	{ .pin = 5, .func = 2 },
> +	{ .pin = 10, .func = 2 },
> +	{ .pin = 11, .func = 2 },
> +	{ .pin = 12, .func = 2 },
> +	{ .pin = 13, .func = 2 },
> +};

TIOC in mode3, SKC3 and SCK1 in mode 5
> +
> +static const struct rza1_bidir_pin rza1l_bidir_pins_p4[] = {
> +	{ .pin = 1, .func = 4 },
> +	{ .pin = 2, .func = 2 },
> +	{ .pin = 3, .func = 2 },
> +	{ .pin = 6, .func = 2 },
> +	{ .pin = 7, .func = 2 },
> +};

Confused, you're here listing TIOC2B pin (pin 1 mode #4) but not TIOC1B (pin 0 mode #4)
ET_MDIO in mode 4, which we list as bidir in RZ/A1H

> +
> +static const struct rza1_bidir_pin rza1l_bidir_pins_p5[] = {
> +	{ .pin = 0, .func = 1 },
> +	{ .pin = 1, .func = 1 },
> +	{ .pin = 2, .func = 1 },
> +	{ .pin = 3, .func = 1 },
> +	{ .pin = 4, .func = 1 },
> +	{ .pin = 5, .func = 1 },
> +	{ .pin = 6, .func = 1 },
> +	{ .pin = 7, .func = 1 },
> +	{ .pin = 8, .func = 1 },
> +	{ .pin = 9, .func = 1 },
> +	{ .pin = 10, .func = 1 },
> +	{ .pin = 11, .func = 1 },
> +	{ .pin = 12, .func = 1 },
> +	{ .pin = 13, .func = 1 },
> +	{ .pin = 14, .func = 1 },
> +	{ .pin = 15, .func = 1 },
> +	{ .pin = 0, .func = 2 },
> +	{ .pin = 1, .func = 2 },
> +	{ .pin = 2, .func = 2 },
> +	{ .pin = 3, .func = 2 },
> +};

I'll stop here with bidir as I guess we're following different documents, as
it happened for RZ/A1H you may have another source of informations

> +
> +static const struct rza1_bidir_pin rza1l_bidir_pins_p6[] = {
> +	{ .pin = 0, .func = 1 },
> +	{ .pin = 1, .func = 1 },
> +	{ .pin = 2, .func = 1 },
> +	{ .pin = 3, .func = 1 },
> +	{ .pin = 4, .func = 1 },
> +	{ .pin = 5, .func = 1 },
> +	{ .pin = 6, .func = 1 },
> +	{ .pin = 7, .func = 1 },
> +	{ .pin = 8, .func = 1 },
> +	{ .pin = 9, .func = 1 },
> +	{ .pin = 10, .func = 1 },
> +	{ .pin = 11, .func = 1 },
> +	{ .pin = 12, .func = 1 },
> +	{ .pin = 13, .func = 1 },
> +	{ .pin = 14, .func = 1 },
> +	{ .pin = 15, .func = 1 },
> +};
> +
> +static const struct rza1_bidir_pin rza1l_bidir_pins_p7[] = {
> +	{ .pin = 2, .func = 2 },
> +	{ .pin = 3, .func = 2 },
> +	{ .pin = 5, .func = 2 },
> +	{ .pin = 6, .func = 2 },
> +	{ .pin = 7, .func = 2 },
> +	{ .pin = 2, .func = 3 },
> +	{ .pin = 3, .func = 3 },
> +	{ .pin = 5, .func = 3 },
> +	{ .pin = 6, .func = 3 },
> +	{ .pin = 7, .func = 3 },
> +};
> +
> +static const struct rza1_bidir_pin rza1l_bidir_pins_p9[] = {
> +	{ .pin = 1, .func = 2 },
> +	{ .pin = 0, .func = 3 },
> +	{ .pin = 1, .func = 3 },
> +	{ .pin = 3, .func = 3 },
> +	{ .pin = 4, .func = 3 },
> +	{ .pin = 5, .func = 3 },
> +};
> +
> +static const struct rza1_swio_pin rza1l_swio_pins[] = {
> +	{ .port = 2, .pin = 8, .func = 2, .input = 0 },
> +	{ .port = 5, .pin = 6, .func = 3, .input = 0 },
> +	{ .port = 6, .pin = 6, .func = 3, .input = 0 },
> +	{ .port = 6, .pin = 10, .func = 3, .input = 0 },
> +	{ .port = 7, .pin = 10, .func = 2, .input = 0 },
> +	{ .port = 8, .pin = 2, .func = 3, .input = 0 },
> +};

I count 20 SWIO pins

TIOC[0-4][A-D]
SSITx[0-3]
WDTOVF

Thanks
   j
> +
> +static const struct rza1_bidir_entry rza1l_bidir_entries[RZA1_NPORTS] = {
> +	[1] = { ARRAY_SIZE(rza1l_bidir_pins_p1), rza1l_bidir_pins_p1 },
> +	[3] = { ARRAY_SIZE(rza1l_bidir_pins_p3), rza1l_bidir_pins_p3 },
> +	[4] = { ARRAY_SIZE(rza1l_bidir_pins_p4), rza1l_bidir_pins_p4 },
> +	[5] = { ARRAY_SIZE(rza1l_bidir_pins_p4), rza1l_bidir_pins_p5 },
> +	[6] = { ARRAY_SIZE(rza1l_bidir_pins_p6), rza1l_bidir_pins_p6 },
> +	[7] = { ARRAY_SIZE(rza1l_bidir_pins_p7), rza1l_bidir_pins_p7 },
> +	[9] = { ARRAY_SIZE(rza1l_bidir_pins_p9), rza1l_bidir_pins_p9 },
> +};
> +
> +static const struct rza1_swio_entry rza1l_swio_entries[] = {
> +	[0] = { ARRAY_SIZE(rza1h_swio_pins), rza1h_swio_pins },
> +};
> +
> +/* RZ/A1L (r7s72102x) pinmux flags table */
> +static const struct rza1_pinmux_conf rza1l_pmx_conf = {
> +	.bidir_entries	= rza1l_bidir_entries,
> +	.swio_entries	= rza1l_swio_entries,
> +};
> +
>  /* ----------------------------------------------------------------------------
>   * RZ/A1 types
>   */
> @@ -1283,9 +1411,15 @@ static int rza1_pinctrl_probe(struct platform_device *pdev)
>
>  static const struct of_device_id rza1_pinctrl_of_match[] = {
>  	{
> +		/* RZ/A1H, RZ/A1M */
>  		.compatible	= "renesas,r7s72100-ports",
>  		.data		= &rza1h_pmx_conf,
>  	},
> +	{
> +		/* RZ/A1L */
> +		.compatible	= "renesas,r7s72102-ports",
> +		.data		= &rza1l_pmx_conf,
> +	},
>  	{ }
>  };
>
> --
> 2.14.1
>
>

RE: [PATCH 1/2] pinctrl: rza1: add support for RZ/A1L

[Chris Brandt]

Hi Jacopo,

On Tuesday, October 03, 2017, jacopo mondi wrote:
> Do you have other authoritative sources for pin's bidir settings?
> According to the publicly available manual, there are other pins that
> needs bidir not listed here.
> As the same happened with RZ/A1H I'm not too concerned, but I'm
> listing some of there here nonetheless

Back when you were working on this, I discussed with the guys back in 
Japan. While in the table there are multiple pins that show 
'bi-directional capable', they don't all need the bi-dir bit set. This was the list of
peripheral:
 I2C, MDIO (ETH), MMC, SDHI, QSPI, Data (D0-D31)


> P2 has several TIOC pins in mode 4 and SSI in mode 2

Port 2???
Port 2, mode 4 in the RZ/A1L manual only has IRQ7,6,5,2,1


I thought we decided on that TIOC pins are ones that are manually set to
either input (input-enable, output-enable) by the users Device Tree, so
they don't need to be put into a table. If input-enable or 
output-enable is specified, then you manually set it to that direction.

As for SSI, the manual says only the SSITx pins need bi-dir, not the 
SSIRx pins.


> TIOC in mode3, SKC3 and SCK1 in mode 5
> > +
> > +static const struct rza1_bidir_pin rza1l_bidir_pins_p4[] = {
> > +	{ .pin = 1, .func = 4 },
> > +	{ .pin = 2, .func = 2 },
> > +	{ .pin = 3, .func = 2 },
> > +	{ .pin = 6, .func = 2 },
> > +	{ .pin = 7, .func = 2 },
> > +};
> 
> Confused, you're here listing TIOC2B pin (pin 1 mode #4) but not TIOC1B
> (pin 0 mode #4)
> ET_MDIO in mode 4, which we list as bidir in RZ/A1H

P4_1 mode 4 is ET_MDIO.

TIOC1B is P4_0 mode 3.
TIOC2B is P4_1 Mode 3

Are you looking at Table 41.19 in the Rev.3.00 version of the RZ/A1L 
Hardware Manual?

> > +static const struct rza1_swio_pin rza1l_swio_pins[] = {
> > +	{ .port = 2, .pin = 8, .func = 2, .input = 0 },
> > +	{ .port = 5, .pin = 6, .func = 3, .input = 0 },
> > +	{ .port = 6, .pin = 6, .func = 3, .input = 0 },
> > +	{ .port = 6, .pin = 10, .func = 3, .input = 0 },
> > +	{ .port = 7, .pin = 10, .func = 2, .input = 0 },
> > +	{ .port = 8, .pin = 2, .func = 3, .input = 0 },
> > +};
> 
> I count 20 SWIO pins
> 
> TIOC[0-4][A-D]
> SSITx[0-3]
> WDTOVF

I thought we said TIOC didn't need to be in a table because the user has
to specify them anyway because the user has to choose if he wants them 
as input or output, so if he does that, then we automatically know it's 
a swio pin. Hmm, maybe I have to go back and look at what the driver 
ended up implementing.


Chris

Re: [PATCH 1/2] pinctrl: rza1: add support for RZ/A1L

[jacopo mondi]

Hi Chris,

On Tue, Oct 03, 2017 at 01:36:57PM +0000, Chris Brandt wrote:
> Hi Jacopo,
>
> On Tuesday, October 03, 2017, jacopo mondi wrote:
> > Do you have other authoritative sources for pin's bidir settings?
> > According to the publicly available manual, there are other pins that
> > needs bidir not listed here.
> > As the same happened with RZ/A1H I'm not too concerned, but I'm
> > listing some of there here nonetheless
>
> Back when you were working on this, I discussed with the guys back in
> Japan. While in the table there are multiple pins that show
> 'bi-directional capable', they don't all need the bi-dir bit set. This was the list of
> peripheral:
>  I2C, MDIO (ETH), MMC, SDHI, QSPI, Data (D0-D31)

Oh, I just received the list of pin in txt format iirc and didn't
noted down which peripherals where actually bi-dir

>
>
> > P2 has several TIOC pins in mode 4 and SSI in mode 2
>
> Port 2???
> Port 2, mode 4 in the RZ/A1L manual only has IRQ7,6,5,2,1

That was mode #3, sorry about this.
>
>
> I thought we decided on that TIOC pins are ones that are manually set to
> either input (input-enable, output-enable) by the users Device Tree, so
> they don't need to be put into a table. If input-enable or
> output-enable is specified, then you manually set it to that direction.
>

Correct

> As for SSI, the manual says only the SSITx pins need bi-dir, not the
> SSIRx pins.

Looking at RZ/A1H tables you're right. I wonder why in RZ/A1L SSI pins
are listed as bidir and in RZ/A1H they're not.
>
>
> > TIOC in mode3, SKC3 and SCK1 in mode 5
> > > +
> > > +static const struct rza1_bidir_pin rza1l_bidir_pins_p4[] = {
> > > +	{ .pin = 1, .func = 4 },
> > > +	{ .pin = 2, .func = 2 },
> > > +	{ .pin = 3, .func = 2 },
> > > +	{ .pin = 6, .func = 2 },
> > > +	{ .pin = 7, .func = 2 },
> > > +};
> >
> > Confused, you're here listing TIOC2B pin (pin 1 mode #4) but not TIOC1B
> > (pin 0 mode #4)
> > ET_MDIO in mode 4, which we list as bidir in RZ/A1H
>
> P4_1 mode 4 is ET_MDIO.

And I keep counting modes from 0...

>
> TIOC1B is P4_0 mode 3.
> TIOC2B is P4_1 Mode 3

So it's fine to leave TIOC out from this table.

>
> Are you looking at Table 41.19 in the Rev.3.00 version of the RZ/A1L
> Hardware Manual?
>
> > > +static const struct rza1_swio_pin rza1l_swio_pins[] = {
> > > +	{ .port = 2, .pin = 8, .func = 2, .input = 0 },
> > > +	{ .port = 5, .pin = 6, .func = 3, .input = 0 },
> > > +	{ .port = 6, .pin = 6, .func = 3, .input = 0 },
> > > +	{ .port = 6, .pin = 10, .func = 3, .input = 0 },
> > > +	{ .port = 7, .pin = 10, .func = 2, .input = 0 },
> > > +	{ .port = 8, .pin = 2, .func = 3, .input = 0 },
> > > +};
> >
> > I count 20 SWIO pins
> >
> > TIOC[0-4][A-D]
> > SSITx[0-3]
> > WDTOVF
>
> I thought we said TIOC didn't need to be in a table because the user has
> to specify them anyway because the user has to choose if he wants them
> as input or output, so if he does that, then we automatically know it's
> a swio pin. Hmm, maybe I have to go back and look at what the driver
> ended up implementing.
>

Can confirm TIOC pins are not listed in SWIO table in RZ/A1H.

Thanks for double checking. Your source from Japan is more
authoritative than manuals and defintely more accurate than me ;)

Thanks
   j

>
> Chris

RE: [PATCH 1/2] pinctrl: rza1: add support for RZ/A1L

[Chris Brandt]

Hi Jacopo,

On Tuesday, October 03, 2017 1, jacopo mondi wrote:
> > As for SSI, the manual says only the SSITx pins need bi-dir, not the
> > SSIRx pins.
> 
> Looking at RZ/A1H tables you're right. I wonder why in RZ/A1L SSI pins
> are listed as bidir and in RZ/A1H they're not.

Sorry, to be clear, SSITx pins have nothing to do with "bi-dir".

Those were pins that you have to add to the rza1l_swio_pins[] array 
because they must have to have both PIPCn.PIPC and PM bits set to '0' (ie, 
forced to output).

However, there is no SSITxD5 pin in RZ/A1L.

  RZ/A1H: SSITxD0, SSITxD1, SSITxD3, SSITxD5
  RZ/A1L: SSITxD0, SSITxD1, SSITxD3



> > I thought we said TIOC didn't need to be in a table because the user has
> > to specify them anyway because the user has to choose if he wants them
> > as input or output, so if he does that, then we automatically know it's
> > a swio pin. Hmm, maybe I have to go back and look at what the driver
> > ended up implementing.
> >
> 
> Can confirm TIOC pins are not listed in SWIO table in RZ/A1H.

Yes, that 'are' listed in the table. But for a good reason.

The TIOC pins can be uses as an output (PWM for example) or as a input 
(input capture for example), so they are not really a fixed direction. 
That is why the user needs to specify how they plan to use them in the DT.
And...that's also why it doesn't make sense to put them in the 
rza1l_swio_pins[] array (we have no idea how they want to use them).

So, the hardware manual has the TIOC pins in the SWIO table to tell the 
user 'hey, you have to manually set these pins to either input or output
yourself because we can't magically know what you want to use them for.

But...
The reason for the rza1l_swio_pins[] array is for weird pin 
configurations that are fixed (like LCD LVDS output pins need to be configured as 
inputs).


> Thanks for double checking. Your source from Japan is more
> authoritative than manuals and defintely more accurate than me ;)

So, after all that, I don't think we found any issues with my patch.
Correct???


Thanks!

Chris

Re: [PATCH 1/2] pinctrl: rza1: add support for RZ/A1L

[jacopo mondi]

Hi Chris,

On Tue, Oct 03, 2017 at 03:40:51PM +0000, Chris Brandt wrote:
> Hi Jacopo,
>
>
> Yes, that 'are' listed in the table. But for a good reason.
>
> The TIOC pins can be uses as an output (PWM for example) or as a input
> (input capture for example), so they are not really a fixed direction.
> That is why the user needs to specify how they plan to use them in the DT.
> And...that's also why it doesn't make sense to put them in the
> rza1l_swio_pins[] array (we have no idea how they want to use them).
>
> So, the hardware manual has the TIOC pins in the SWIO table to tell the
> user 'hey, you have to manually set these pins to either input or output
> yourself because we can't magically know what you want to use them for.
>
> But...
> The reason for the rza1l_swio_pins[] array is for weird pin
> configurations that are fixed (like LCD LVDS output pins need to be configured as
> inputs).
>
>
> > Thanks for double checking. Your source from Japan is more
> > authoritative than manuals and defintely more accurate than me ;)
>
> So, after all that, I don't think we found any issues with my patch.
> Correct???

Correct! :)
After your clarification above, for both patches:

Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
   j

>
>
> Thanks!
>
> Chris
>

[PATCH 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Chris Brandt]

Compatible "r7s72102-ports" was added for RZ/A1L devices

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
index 43e21474528a..2a3366cee848 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
@@ -13,7 +13,8 @@ Pin controller node
 
 Required properties:
   - compatible
-    this shall be "renesas,r7s72100-ports".
+    this shall be "renesas,r7s72100-ports" for RZ/A1H and RZ/A1M or
+    "renesas,r7s72102-ports" for RZ/A1L
 
   - reg
     address base and length of the memory area where the pin controller
-- 
2.14.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Geert Uytterhoeven]

Hi Chris,

On Tue, Oct 3, 2017 at 9:33 AM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Compatible "r7s72102-ports" was added for RZ/A1L devices
>
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> index 43e21474528a..2a3366cee848 100644
> --- a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> @@ -13,7 +13,8 @@ Pin controller node
>
>  Required properties:
>    - compatible
> -    this shall be "renesas,r7s72100-ports".
> +    this shall be "renesas,r7s72100-ports" for RZ/A1H and RZ/A1M or
> +    "renesas,r7s72102-ports" for RZ/A1L

Shouldn't you also document "renesas,r7s72101-ports" for RZ/A1M?
You could use "renesas,r7s72100-ports" as a fallback property for RZ/A1M,
though.

BTW, is there a way to distinguish the various RZ/A1 SoCs at runtime?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Chris Brandt]

Hi Geert,

On Wednesday, October 04, 2017, Geert Uytterhoeven wrote:
> >  Required properties:
> >    - compatible
> > -    this shall be "renesas,r7s72100-ports".
> > +    this shall be "renesas,r7s72100-ports" for RZ/A1H and RZ/A1M or
> > +    "renesas,r7s72102-ports" for RZ/A1L
> 
> Shouldn't you also document "renesas,r7s72101-ports" for RZ/A1M?

I was wondering if you were going to ask for that.

I put "for RZ/A1H and RZ/A1M" in the DT documentation, but if you 
prefer, I can also just add to pinctrl-rza1.c:

static const struct of_device_id rza1_pinctrl_of_match[] = {
	{
		/* RZ/A1H */
		.compatible	= "renesas,r7s72100-ports",
		.data		= &rza1h_pmx_conf,
	},
+	{
+		/* RZ/A1M */
+		.compatible	= "renesas,r7s72101-ports",
+		.data		= &rza1h_pmx_conf,
+	},	{
		/* RZ/A1L */
		.compatible	= "renesas,r7s72102-ports",
		.data		= &rza1l_pmx_conf,
	},
	{ }
};



> You could use "renesas,r7s72100-ports" as a fallback property for RZ/A1M,
> though.

Do you mean in the DT? Or in pinctrl-rza1.c?

> BTW, is there a way to distinguish the various RZ/A1 SoCs at runtime?

Sadly there is not.

There is a unique ID number, but it is only accessible via JTAG 
externally.

Internally, you would have to look for registers that don't exist (for 
the RZ/A1L). But for RZ/A1H vs RZ/A1M, the only actual difference is the 
amount of RAM, so you would have to do a set-and-test method on RAM.


Chris

Re: [PATCH 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Geert Uytterhoeven]

Hi Chris,

On Wed, Oct 4, 2017 at 4:16 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, October 04, 2017, Geert Uytterhoeven wrote:
>> >  Required properties:
>> >    - compatible
>> > -    this shall be "renesas,r7s72100-ports".
>> > +    this shall be "renesas,r7s72100-ports" for RZ/A1H and RZ/A1M or
>> > +    "renesas,r7s72102-ports" for RZ/A1L
>>
>> Shouldn't you also document "renesas,r7s72101-ports" for RZ/A1M?
>
> I was wondering if you were going to ask for that.

:-)

> I put "for RZ/A1H and RZ/A1M" in the DT documentation, but if you
> prefer, I can also just add to pinctrl-rza1.c:
>
> static const struct of_device_id rza1_pinctrl_of_match[] = {
>         {
>                 /* RZ/A1H */
>                 .compatible     = "renesas,r7s72100-ports",
>                 .data           = &rza1h_pmx_conf,
>         },
> +       {
> +               /* RZ/A1M */
> +               .compatible     = "renesas,r7s72101-ports",
> +               .data           = &rza1h_pmx_conf,
> +       },      {
>                 /* RZ/A1L */
>                 .compatible     = "renesas,r7s72102-ports",
>                 .data           = &rza1l_pmx_conf,
>         },
>         { }
> };
>
>
>
>> You could use "renesas,r7s72100-ports" as a fallback property for RZ/A1M,
>> though.
>
> Do you mean in the DT? Or in pinctrl-rza1.c?

I mean in DT, i.e.:

        pinctrl: pin-controller@fcfe3000 {
                compatible = "renesas,r7s72101-ports", "renesas,r7s72100-ports";
                ...

That way you don't have to extend the pinctrl driver.

>> BTW, is there a way to distinguish the various RZ/A1 SoCs at runtime?
>
> Sadly there is not.
>
> There is a unique ID number, but it is only accessible via JTAG
> externally.
>
> Internally, you would have to look for registers that don't exist (for
> the RZ/A1L). But for RZ/A1H vs RZ/A1M, the only actual difference is the
> amount of RAM, so you would have to do a set-and-test method on RAM.

If RZ/A1H and RZ/A1M differ in the amount of RAM only, I think you can
just consider them the same, though, and use the same DTS (assumed
the boot loader fills/corrects in the memory node).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Chris Brandt]

Hi Geert,

To be clear:

On Wednesday, October 04, 2017 1, Geert Uytterhoeven wrote:
> >> You could use "renesas,r7s72100-ports" as a fallback property for
> RZ/A1M,
> >> though.
> >
> > Do you mean in the DT? Or in pinctrl-rza1.c?
> 
> I mean in DT, i.e.:
> 
>         pinctrl: pin-controller@fcfe3000 {
>                 compatible = "renesas,r7s72101-ports", "renesas,r7s72100-
> ports";
>                 ...
> 
> That way you don't have to extend the pinctrl driver.

I will submit a v2 with:

[ renesas,rza1-pinctrl.txt ]

Required properties:
  - compatible
    this shall be "renesas,r7s72100-ports" for RZ/A1H, 
    "renesas,r7s72101-ports" for RZ/M or
    "renesas,r7s72102-ports" for RZ/A1L


[ r7s72100.dtsi ]
	pinctrl: pin-controller@fcfe3000 {
		compatible = "renesas,r7s72101-ports", "renesas,r7s72100-ports";


[ pinctrl-rza1.c ] 
  No change from V1



Unless you say something different, I will submit a V2 in a little 
while.

As always, thank you for your guidance!

Chris

Re: [PATCH 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Geert Uytterhoeven]

Hi Chris,

On Wed, Oct 4, 2017 at 4:43 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> To be clear:
>
> On Wednesday, October 04, 2017 1, Geert Uytterhoeven wrote:
>> >> You could use "renesas,r7s72100-ports" as a fallback property for
>> RZ/A1M,
>> >> though.
>> >
>> > Do you mean in the DT? Or in pinctrl-rza1.c?
>>
>> I mean in DT, i.e.:
>>
>>         pinctrl: pin-controller@fcfe3000 {
>>                 compatible = "renesas,r7s72101-ports", "renesas,r7s72100-
>> ports";
>>                 ...
>>
>> That way you don't have to extend the pinctrl driver.
>
> I will submit a v2 with:
>
> [ renesas,rza1-pinctrl.txt ]
>
> Required properties:
>   - compatible
>     this shall be "renesas,r7s72100-ports" for RZ/A1H,
>     "renesas,r7s72101-ports" for RZ/M or

... with a fallback for "renesas,r7s72100-ports, as RZ/A1M is compatible
with RZ/A1H (and I assume RZ/A1H is the predecessor?).

>     "renesas,r7s72102-ports" for RZ/A1L
>
>
> [ r7s72100.dtsi ]
>         pinctrl: pin-controller@fcfe3000 {
>                 compatible = "renesas,r7s72101-ports", "renesas,r7s72100-ports";

I meant in r7s72101.dtsi (which we don't have!).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 2/2] dt-bindings: pinctrl: add support for RZ/A1L

[Chris Brandt]

Hi Geert,

On Wednesday, October 04, 2017 1, Geert Uytterhoeven wrote:
> > I will submit a v2 with:
> >
> > [ renesas,rza1-pinctrl.txt ]
> >
> > Required properties:
> >   - compatible
> >     this shall be "renesas,r7s72100-ports" for RZ/A1H,
> >     "renesas,r7s72101-ports" for RZ/M or
> 
> ... with a fallback for "renesas,r7s72100-ports, as RZ/A1M is compatible
> with RZ/A1H (and I assume RZ/A1H is the predecessor?).

OK.

(and yes, RZ/A1H was the predecessor of RZ/A1M)


> >     "renesas,r7s72102-ports" for RZ/A1L
> >
> >
> > [ r7s72100.dtsi ]
> >         pinctrl: pin-controller@fcfe3000 {
> >                 compatible = "renesas,r7s72101-ports",
> "renesas,r7s72100-ports";
> 
> I meant in r7s72101.dtsi (which we don't have!).

Ohhhhh, I see  :)


Thank you.

Chris