All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukasz Majewski <lukma@denx.de>
To: Shawn Guo <shawnguo@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Sascha Hauer <s.hauer@pengutronix.de>,
	linux-kernel@vger.kernel.org, Stefan Agner <stefan@agner.ch>,
	Rob Herring <robh+dt@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Fabio Estevam <festevam@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: dts: Provide support for reading ID code from MVB device (BK4)
Date: Mon, 3 Dec 2018 10:33:33 +0100	[thread overview]
Message-ID: <20181203103333.111c2463@jawa> (raw)
In-Reply-To: <20181126143619.GB31334@dragon>


[-- Attachment #1.1: Type: text/plain, Size: 3183 bytes --]

Hi Shawn,

Thank you for the review.

> On Tue, Nov 13, 2018 at 01:12:13PM +0100, Lukasz Majewski wrote:
> > The procedure to read this ID value is as follows:
> > 
> > rmmod spi_fsl_dspi
> > insmod spi-gpio.ko
> > 
> > echo 504 > /sys/class/gpio/export
> > cat /sys/class/gpio/gpio504/value
> > ...
> > echo 511 > /sys/class/gpio/export
> > cat /sys/class/gpio/gpio511/value
> > 
> > rmmod spi-gpio.ko
> > insmod spi_fsl_dspi
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> A prefix like 'ARM: dts: vf610-bk4: ...' might be better.

Ok.

> 
> > ---
> >  arch/arm/boot/dts/vf610-bk4.dts | 31
> > +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/vf610-bk4.dts
> > b/arch/arm/boot/dts/vf610-bk4.dts index cab95714c058..f01c735807ae
> > 100644 --- a/arch/arm/boot/dts/vf610-bk4.dts
> > +++ b/arch/arm/boot/dts/vf610-bk4.dts
> > @@ -59,6 +59,29 @@
> >  		regulator-min-microvolt = <3300000>;
> >  		regulator-max-microvolt = <3300000>;
> >  	};
> > +
> > +	spi_gpio {
> 
> We recommend hyphen rather than underscore be used in node name.

Ok. I will change it to spi-gpio

> 
> > +		compatible = "spi-gpio";
> > +		pinctrl-0 = <&pinctrl_gpio_spi>;
> > +		pinctrl-names = "default";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		/* PTD12 ->RPIO[91] */
> > +		sck-gpios  = <&gpio2 27 GPIO_ACTIVE_LOW>;
> > +		/* PTD10 ->RPIO[89] */
> > +		miso-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>;
> > +		num-chipselects = <0>;
> > +
> > +		72xx165@0 {
> 
> Please use a generic name for the node.

This is a bit tricky.

Other DTS definitions for this compatible:

sn65hvs882: sn65hvs882@0 {
	compatible = "pisosr-gpio";
	...
	}

They use the exact used IC name - sn65hvs882, which IMHO is the way how
this node shall be described.

The reason is that the compatible is "pisosr-gpio" -> parallel input
serial output shift regsiter - gpio.

By adding the exact name of connected electronic IC - sn65hvs882 and in
my case IC from the 72xx165 family provides good description of the HW.

Maybe you have other idea how to provide the name of the IC connected?

> 
> Shawn
> 
> > +			compatible = "pisosr-gpio";
> > +			reg = <0>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			/* PTB18 -> RGPIO[40] */
> > +			load-gpios  = <&gpio1 8 GPIO_ACTIVE_LOW>;
> > +			spi-max-frequency = <100000>;
> > +		};
> > +	};
> >  };
> >  
> >  &adc0 {
> > @@ -430,6 +453,14 @@
> >  		>;
> >  	};
> >  
> > +	pinctrl_gpio_spi: pinctrl-gpio-spi {
> > +		fsl,pins = <
> > +			VF610_PAD_PTB18__GPIO_40        0x1183
> > +			VF610_PAD_PTD10__GPIO_89        0x1183
> > +			VF610_PAD_PTD12__GPIO_91        0x1183
> > +		>;
> > +	};
> > +
> >  	pinctrl_i2c2: i2c2grp {
> >  		fsl,pins = <
> >  			VF610_PAD_PTA22__I2C2_SCL
> > 0x34df -- 
> > 2.11.0
> > 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Lukasz Majewski <lukma@denx.de>
To: Shawn Guo <shawnguo@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Stefan Agner <stefan@agner.ch>,
	Fabio Estevam <festevam@gmail.com>,
	Fabio Estevam <fabio.estevam@nxp.com>
Subject: Re: [PATCH] ARM: dts: Provide support for reading ID code from MVB device (BK4)
Date: Mon, 3 Dec 2018 10:33:33 +0100	[thread overview]
Message-ID: <20181203103333.111c2463@jawa> (raw)
In-Reply-To: <20181126143619.GB31334@dragon>

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

Hi Shawn,

Thank you for the review.

> On Tue, Nov 13, 2018 at 01:12:13PM +0100, Lukasz Majewski wrote:
> > The procedure to read this ID value is as follows:
> > 
> > rmmod spi_fsl_dspi
> > insmod spi-gpio.ko
> > 
> > echo 504 > /sys/class/gpio/export
> > cat /sys/class/gpio/gpio504/value
> > ...
> > echo 511 > /sys/class/gpio/export
> > cat /sys/class/gpio/gpio511/value
> > 
> > rmmod spi-gpio.ko
> > insmod spi_fsl_dspi
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> A prefix like 'ARM: dts: vf610-bk4: ...' might be better.

Ok.

> 
> > ---
> >  arch/arm/boot/dts/vf610-bk4.dts | 31
> > +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/vf610-bk4.dts
> > b/arch/arm/boot/dts/vf610-bk4.dts index cab95714c058..f01c735807ae
> > 100644 --- a/arch/arm/boot/dts/vf610-bk4.dts
> > +++ b/arch/arm/boot/dts/vf610-bk4.dts
> > @@ -59,6 +59,29 @@
> >  		regulator-min-microvolt = <3300000>;
> >  		regulator-max-microvolt = <3300000>;
> >  	};
> > +
> > +	spi_gpio {
> 
> We recommend hyphen rather than underscore be used in node name.

Ok. I will change it to spi-gpio

> 
> > +		compatible = "spi-gpio";
> > +		pinctrl-0 = <&pinctrl_gpio_spi>;
> > +		pinctrl-names = "default";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		/* PTD12 ->RPIO[91] */
> > +		sck-gpios  = <&gpio2 27 GPIO_ACTIVE_LOW>;
> > +		/* PTD10 ->RPIO[89] */
> > +		miso-gpios = <&gpio2 25 GPIO_ACTIVE_HIGH>;
> > +		num-chipselects = <0>;
> > +
> > +		72xx165@0 {
> 
> Please use a generic name for the node.

This is a bit tricky.

Other DTS definitions for this compatible:

sn65hvs882: sn65hvs882@0 {
	compatible = "pisosr-gpio";
	...
	}

They use the exact used IC name - sn65hvs882, which IMHO is the way how
this node shall be described.

The reason is that the compatible is "pisosr-gpio" -> parallel input
serial output shift regsiter - gpio.

By adding the exact name of connected electronic IC - sn65hvs882 and in
my case IC from the 72xx165 family provides good description of the HW.

Maybe you have other idea how to provide the name of the IC connected?

> 
> Shawn
> 
> > +			compatible = "pisosr-gpio";
> > +			reg = <0>;
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			/* PTB18 -> RGPIO[40] */
> > +			load-gpios  = <&gpio1 8 GPIO_ACTIVE_LOW>;
> > +			spi-max-frequency = <100000>;
> > +		};
> > +	};
> >  };
> >  
> >  &adc0 {
> > @@ -430,6 +453,14 @@
> >  		>;
> >  	};
> >  
> > +	pinctrl_gpio_spi: pinctrl-gpio-spi {
> > +		fsl,pins = <
> > +			VF610_PAD_PTB18__GPIO_40        0x1183
> > +			VF610_PAD_PTD10__GPIO_89        0x1183
> > +			VF610_PAD_PTD12__GPIO_91        0x1183
> > +		>;
> > +	};
> > +
> >  	pinctrl_i2c2: i2c2grp {
> >  		fsl,pins = <
> >  			VF610_PAD_PTA22__I2C2_SCL
> > 0x34df -- 
> > 2.11.0
> > 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2018-12-03  9:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-13 12:12 [PATCH] ARM: dts: Provide support for reading ID code from MVB device (BK4) Lukasz Majewski
2018-11-13 12:12 ` Lukasz Majewski
2018-11-26 14:36 ` Shawn Guo
2018-11-26 14:36   ` Shawn Guo
2018-12-03  9:33   ` Lukasz Majewski [this message]
2018-12-03  9:33     ` Lukasz Majewski
2018-12-03 11:13     ` Fabio Estevam
2018-12-03 11:13       ` Fabio Estevam
2018-12-05 16:40       ` Lukasz Majewski
2018-12-05 16:40         ` Lukasz Majewski
2018-12-09 21:50 ` [PATCH v2] ARM: dts: vf610-bk4: Provide support for reading ID code from MVB device Lukasz Majewski
2018-12-09 21:50   ` Lukasz Majewski
2019-01-10 13:22   ` Shawn Guo
2019-01-10 13:22     ` Shawn Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181203103333.111c2463@jawa \
    --to=lukma@denx.de \
    --cc=devicetree@vger.kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=stefan@agner.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.