Devicetree
 help / color / mirror / Atom feed
* Re: [PATCHv2 2/3] ARM: drm: Intel FPGA VIP Frame Buffer II drm driver
From: Jani Nikula @ 2017-04-25  7:17 UTC (permalink / raw)
  To: daniel.vetter-ral2JQCrhuEAvxtiuMwx3w,
	dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: hean.loong.ong-ral2JQCrhuEAvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	tien.hock.loh-ral2JQCrhuEAvxtiuMwx3w, Ong-CC+yJ3UmIYqDUpFQwHEjaQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <1493086006-4392-3-git-send-email-hean.loong.ong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On Tue, 25 Apr 2017, hean.loong.ong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> +++ b/drivers/gpu/drm/ivip/Makefile
> @@ -0,0 +1,9 @@
> +#
> +# Makefile for the drm device driver.  This driver provides support for the
> +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> +
> +ccflags-y := -Iinclude/drm

Just a drive-by observation, there are patches on the list removing such
ccflags from existing drivers. You shouldn't need this. Just make sure
all your #includes begin with <drm/.

BR,
Jani.

> +
> +obj-$(CONFIG_DRM_IVIP) += ivip.o
> +ivip-objs := intel_vip_of.o intel_vip_core.o \
> +intel_vip_conn.o
-- 
Jani Nikula, Intel Open Source Technology Center
--
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

^ permalink raw reply

* Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver
From: Geert Uytterhoeven @ 2017-04-25  7:15 UTC (permalink / raw)
  To: zoobab
  Cc: Linus Walleij, Nandor Han, Greg KH, David S. Miller,
	Mauro Carvalho Chehab, Daniel Vetter, Alexandre Courbot,
	Rob Herring, Mark Rutland, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Semi Malinen
In-Reply-To: <CANjd3ndZkVi5OA0eqDewrpqVLQpT11QbVOA98F=ghMhcwPu2hQ@mail.gmail.com>

Hi Benjamin,

On Tue, Apr 25, 2017 at 9:07 AM, Benjamin Henrion <zoobab@gmail.com> wrote:
> On Mon, Apr 24, 2017 at 3:47 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Thu, Apr 13, 2017 at 12:27 PM, Nandor Han <nandor.han@ge.com> wrote:
>>
>>> This is a simple driver that provides a /sys/class/gpio
>>> interface for controlling and configuring the GPIO lines.
>>> It does not provide support for chip select or interrupts.
>>>
>>> Signed-off-by: Nandor Han <nandor.han@ge.com>
>>> Signed-off-by: Semi Malinen <semi.malinen@ge.com>
>>
>> I almost want to make the driver depend on !GPIO_SYSFS because
>> of this commit message.
>>
>> DO NOT USE OR ENCOURAGE THE USE OF THE GPIO SYSFS
>> INTERFACE.
>>
>> Use the character device.
>
> I doubt you will be able to convince the majority of people toggling
> GPIOs via a simple shell script to switch to write a complex C
> program. Not to mention cross compilation and the libraries
> dependencies here.
>
> Is there some good cli tools to access the new char device? If they
> are shipped with most distros, that would reduce the pain.

https://github.com/brgl/libgpiod

A bit early to expect it to be shipped with all distros, though.

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

^ permalink raw reply

* Re: [PATCH] arm64: dts: exynos: Remove the te-gpios property in the TM2 boards
From: Inki Dae @ 2017-04-25  7:12 UTC (permalink / raw)
  To: Hoegeun Kwon, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, kgene-DgEjT+Ai2ygdnm+yROfE0A,
	krzk-DgEjT+Ai2ygdnm+yROfE0A
  Cc: javier-JPH+aEBZ4P+UEJcrhfAQsw, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	a.hajda-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <1493085261-3488-1-git-send-email-hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>



2017년 04월 25일 10:54에 Hoegeun Kwon 이(가) 쓴 글:
> The decon uses HW-TRIGGER, so TE interrupt is not necessary.
> Therefore, remove the te-gpios property in the TM2 dts.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index 3ff9527..23191eb 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -60,7 +60,6 @@
>  		vci-supply = <&ldo28_reg>;
>  		reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
>  		enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
> -		te-gpios = <&gpf1 3 GPIO_ACTIVE_HIGH>;

Reviewed-by: Inki Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Thanks,
Inki Dae

>  	};
>  };
>  
> 
--
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

^ permalink raw reply

* Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver
From: Benjamin Henrion @ 2017-04-25  7:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Nandor Han, Greg KH, David S. Miller, Geert Uytterhoeven,
	Mauro Carvalho Chehab, Daniel Vetter, Alexandre Courbot,
	Rob Herring, Mark Rutland, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Semi Malinen
In-Reply-To: <CACRpkdZHMWH2W-i4MGZ7NKzPHUURRmC-5YwiK7cVLCHKDsb7sw@mail.gmail.com>

On Mon, Apr 24, 2017 at 3:47 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Apr 13, 2017 at 12:27 PM, Nandor Han <nandor.han@ge.com> wrote:
>
>> This is a simple driver that provides a /sys/class/gpio
>> interface for controlling and configuring the GPIO lines.
>> It does not provide support for chip select or interrupts.
>>
>> Signed-off-by: Nandor Han <nandor.han@ge.com>
>> Signed-off-by: Semi Malinen <semi.malinen@ge.com>
>
> I almost want to make the driver depend on !GPIO_SYSFS because
> of this commit message.
>
> DO NOT USE OR ENCOURAGE THE USE OF THE GPIO SYSFS
> INTERFACE.
>
> Use the character device.

I doubt you will be able to convince the majority of people toggling
GPIOs via a simple shell script to switch to write a complex C
program. Not to mention cross compilation and the libraries
dependencies here.

Is there some good cli tools to access the new char device? If they
are shipped with most distros, that would reduce the pain.

Best,

--
Benjamin Henrion <bhenrion at ffii.org>
FFII Brussels - +32-484-566109 - +32-2-3500762
"In July 2005, after several failed attempts to legalise software
patents in Europe, the patent establishment changed its strategy.
Instead of explicitly seeking to sanction the patentability of
software, they are now seeking to create a central European patent
court, which would establish and enforce patentability rules in their
favor, without any possibility of correction by competing courts or
democratically elected legislators."

^ permalink raw reply

* Re: [PATCH 11/13] phy: Add an USB PHY driver for the Lantiq SoCs using the RCU module
From: Hauke Mehrtens @ 2017-04-25  7:06 UTC (permalink / raw)
  To: Rob Herring
  Cc: ralf-6z/3iImG2C8G8FEW9MqTrA, linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg,
	john-Pj+rj9U5foFAfugRpC6u6w, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	hauke.mehrtens-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20170420153606.fdhedc7ovvhc66qd@rob-hp-laptop>



On 04/20/2017 05:36 PM, Rob Herring wrote:
> On Mon, Apr 17, 2017 at 09:29:40PM +0200, Hauke Mehrtens wrote:
>> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> This driver starts the DWC2 core(s) built into the XWAY SoCs and provides
>> the PHY interfaces for each core. The phy instances can be passed to the
>> dwc2 driver, which already supports the generic phy interface.
>>
>> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
>> ---
>>  .../bindings/phy/phy-lantiq-rcu-usb2.txt           |  59 ++++
>>  arch/mips/lantiq/xway/reset.c                      |  43 ---
>>  arch/mips/lantiq/xway/sysctrl.c                    |  24 +-
>>  drivers/phy/Kconfig                                |   8 +
>>  drivers/phy/Makefile                               |   1 +
>>  drivers/phy/phy-lantiq-rcu-usb2.c                  | 325 +++++++++++++++++++++
>>  6 files changed, 405 insertions(+), 55 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt
>>  create mode 100644 drivers/phy/phy-lantiq-rcu-usb2.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt b/Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt
>> new file mode 100644
>> index 000000000000..0ec9f790b6e0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt
>> @@ -0,0 +1,59 @@
>> +Lantiq XWAY SoC RCU USB 1.1/2.0 PHY binding
>> +===========================================
>> +
>> +This binding describes the USB PHY hardware provided by the RCU module on the
>> +Lantiq XWAY SoCs.
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Required properties (controller (parent) node):
>> +- compatible		: Should be one of
>> +				"lantiq,ase-rcu-usb2-phy"
>> +				"lantiq,danube-rcu-usb2-phy"
>> +				"lantiq,xrx100-rcu-usb2-phy"
>> +				"lantiq,xrx200-rcu-usb2-phy"
>> +				"lantiq,xrx300-rcu-usb2-phy"
> 
> The first x in xrx seems to be a wildcard. Don't use wildcards in 
> compatible strings.

Yes that is correct, I will replace it in the newly introduced
compatible strings with the full names without wild cards.

> 
>> +- lantiq,rcu-syscon	: A phandle to the RCU module and the offsets to the
>> +			  USB PHY configuration and USB MAC registers.
> 
> Same comment as gphy.
> 
>> +- address-cells		: should be 1
>> +- size-cells		: should be 0
>> +- phy-cells		: from the generic PHY bindings, must be 1
> 
> Missing the '#'
> 
>> +
>> +Optional properties (controller (parent) node):
>> +- vbus-gpio		: References a GPIO which enables VBUS all given USB
>> +			  ports.
> 
> -gpios is preferred form.
> 
>> +
>> +Required nodes		:  A sub-node is required for each USB PHY port.
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Required properties (port (child) node):
> 
> Where's the sub nodes in the example?

Sorry, this was from an older version, I will update this.

> 
>> +- reg        	: The ID of the USB port, usually 0 or 1.
>> +- clocks	: References to the (PMU) "ctrl" and "phy" clk gates.
>> +- clock-names	: Must be one of the following:
>> +			"ctrl"
>> +			"phy"
>> +- resets	: References to the RCU USB configuration reset bits.
>> +- reset-names	: Must be one of the following:
>> +			"analog-config" (optional)
>> +			"statemachine-soft" (optional)
>> +
>> +Optional properties (port (child) node):
>> +- vbus-gpio	: References a GPIO which enables VBUS for the USB port.
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Example for the USB PHYs on an xRX200 SoC:
>> +	usb_phys0: rcu-usb2-phy@0 {
> 
> usb-phy@...
> 
>> +		compatible      = "lantiq,xrx200-rcu-usb2-phy";
> 
> Extra spaces.
> 
>> +		reg = <0>;
>> +
>> +		lantiq,rcu-syscon = <&rcu0 0x18 0x38>;
>> +		clocks = <&pmu PMU_GATE_USB0_CTRL>,
>> +			 <&pmu PMU_GATE_USB0_PHY>;
>> +		clock-names = "ctrl", "phy";
>> +		vbus-gpios = <&gpio 32 GPIO_ACTIVE_HIGH>;
>> +		resets = <&rcu_reset1 4>, <&rcu_reset0 4>;
>> +		reset-names = "phy", "ctrl";
>> +		#phy-cells = <0>;
>> +	};
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 09/13] MIPS: lantiq: Add a GPHY driver which uses the RCU syscon-mfd
From: Hauke Mehrtens @ 2017-04-25  7:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: ralf-6z/3iImG2C8G8FEW9MqTrA, linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg,
	john-Pj+rj9U5foFAfugRpC6u6w, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	hauke.mehrtens-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20170420152754.3tkjxjvoiuatbvpo@rob-hp-laptop>



On 04/20/2017 05:27 PM, Rob Herring wrote:
> On Mon, Apr 17, 2017 at 09:29:38PM +0200, Hauke Mehrtens wrote:
>> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> Compared to the old xrx200_phy_fw driver the new version has multiple
>> enhancements. The name of the firmware files does not have to be added
>> to all .dts files anymore - one now configures the GPHY mode (FE or GE)
>> instead. Each GPHY can now also boot separate firmware (thus mixing of
>> GE and FE GPHYs is now possible).
>> The new implementation is based on the RCU syscon-mfd and uses the
>> reeset_controller framework instead of raw RCU register reads/writes.
>>
>> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
>> ---
>>  .../devicetree/bindings/mips/lantiq/rcu-gphy.txt   |  54 +++++
>>  arch/mips/lantiq/xway/sysctrl.c                    |   4 +-
>>  drivers/soc/lantiq/Makefile                        |   1 +
>>  drivers/soc/lantiq/gphy.c                          | 242 +++++++++++++++++++++
>>  include/dt-bindings/mips/lantiq_rcu_gphy.h         |  15 ++
>>  5 files changed, 314 insertions(+), 2 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mips/lantiq/rcu-gphy.txt
>>  create mode 100644 drivers/soc/lantiq/gphy.c
>>  create mode 100644 include/dt-bindings/mips/lantiq_rcu_gphy.h
>>
>> diff --git a/Documentation/devicetree/bindings/mips/lantiq/rcu-gphy.txt b/Documentation/devicetree/bindings/mips/lantiq/rcu-gphy.txt
>> new file mode 100644
>> index 000000000000..d525c7ce9f0b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mips/lantiq/rcu-gphy.txt
>> @@ -0,0 +1,54 @@
>> +Lantiq XWAY SoC GPHY binding
>> +============================
>> +
>> +This binding describes a software-defined ethernet PHY, provided by the RCU
>> +module on newer Lantiq XWAY SoCs (xRX200 and newer).
>> +This depends on binary firmware blobs which must be provided by userspace.
> 
> Where the blobs come from is not relevant. 
> 
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Required properties (controller (parent) node):
>> +- compatible		: Should be one of
>> +				"lantiq,xrx200a1x-rcu-gphy"
>> +				"lantiq,xrx200a2x-rcu-gphy"
>> +				"lantiq,xrx300-rcu-gphy"
>> +				"lantiq,xrx330-rcu-gphy"
>> +- lantiq,rcu-syscon	: A phandle and offset to the GPHY address registers in
>> +			  the RCU
>> +- resets		: Must reference the RCU GPHY reset bit
>> +- reset-names		: One entry, value must be "gphy" or optional "gphy2"
>> +
>> +Optional properties (port (child) node):
>> +- lantiq,gphy-mode	: GPHY_MODE_GE (default) or GPHY_MODE_FE as defined in
>> +			  <dt-bindings/mips/lantiq_xway_gphy.h>
>> +- clocks		: A reference to the (PMU) GPHY clock gate
>> +- clock-names		: If clocks is given then this must be "gphy"
> 
> Kind of pointless to have a name for a single clock.

The documentation misses the 2. clock. ;-) Will add it.
> 
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Example for the GPHys on the xRX200 SoCs:
>> +
>> +#include <dt-bindings/mips/lantiq_rcu_gphy.h>
>> +	gphy0: rcu_gphy@0 {
> 
> Use generic node names: phy@...

I will change this

> 
>> +		compatible = "lantiq,xrx200a2x-rcu-gphy";
>> +		reg = <0>;
>> +
>> +		lantiq,rcu-syscon = <&rcu0 0x20>;
> 
> Could the phy just be a child of the rcu? Then you don't need a phandle 
> here and 0x20 becomes the reg address.

The RCU is a register block which does many things. This register is
specific to this ghpy, but there are some register in the RCU block
which are shared between multiple drivers. Can I support both, provide
some parts of this block as syscon and some as direct register blocks?

> 
>> +		resets = <&rcu_reset0 31>, <&rcu_reset1 7>;
>> +		reset-names = "gphy", "gphy2";
>> +		lantiq,gphy-mode = <GPHY_MODE_GE>;
>> +		clocks = <&pmu0 XRX200_PMU_GATE_GPHY>;
>> +		clock-names = "gphy";
>> +	};
>> +
>> +	gphy1: rcu_gphy@1 {
>> +		compatible = "lantiq,xrx200a2x-rcu-gphy";
>> +		reg = <0>;
>> +
>> +		lantiq,rcu-syscon = <&rcu0 0x68>;
>> +		resets = <&rcu_reset0 29>, <&rcu_reset1 6>;
>> +		reset-names = "gphy", "gphy2";
>> +		lantiq,gphy-mode = <GPHY_MODE_FE>;
>> +		clocks = <&pmu0 XRX200_PMU_GATE_GPHY>;
>> +		clock-names = "gphy";
>> +	};
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 08/13] reset: Add a reset controller driver for the Lantiq XWAY based SoCs
From: Hauke Mehrtens @ 2017-04-25  7:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: ralf-6z/3iImG2C8G8FEW9MqTrA, linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg,
	john-Pj+rj9U5foFAfugRpC6u6w, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	hauke.mehrtens-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20170420145405.7s3iapxggr5575d2@rob-hp-laptop>



On 04/20/2017 04:54 PM, Rob Herring wrote:
> On Mon, Apr 17, 2017 at 09:29:37PM +0200, Hauke Mehrtens wrote:
>> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> The reset controllers (on xRX200 and newer SoCs have two of them) are
>> provided by the RCU module. This was initially implemented as a simple
>> reset controller. However, the RCU module provides more functionality
>> (ethernet GPHYs, USB PHY, etc.), which makes it a MFD device.
>> The old reset controller driver implementation from
>> arch/mips/lantiq/xway/reset.c did not honor this fact.
>>
>> For some devices the request and the status bits are different.
>>
>> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
>> ---
>>  .../devicetree/bindings/reset/lantiq,rcu-reset.txt |  43 ++++
>>  arch/mips/lantiq/xway/reset.c                      |  68 ------
>>  drivers/reset/Kconfig                              |   6 +
>>  drivers/reset/Makefile                             |   1 +
>>  drivers/reset/reset-lantiq-rcu.c                   | 231 +++++++++++++++++++++
>>  5 files changed, 281 insertions(+), 68 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/reset/lantiq,rcu-reset.txt
>>  create mode 100644 drivers/reset/reset-lantiq-rcu.c
>>
>> diff --git a/Documentation/devicetree/bindings/reset/lantiq,rcu-reset.txt b/Documentation/devicetree/bindings/reset/lantiq,rcu-reset.txt
>> new file mode 100644
>> index 000000000000..7f097d16bbb7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/lantiq,rcu-reset.txt
>> @@ -0,0 +1,43 @@
>> +Lantiq XWAY SoC RCU reset controller binding
>> +============================================
>> +
>> +This binding describes a reset-controller found on the RCU module on Lantiq
>> +XWAY SoCs.
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Required properties (controller (parent) node):
>> +- compatible		: Should be "lantiq,rcu-reset"
>> +- lantiq,rcu-syscon	: A phandle to the RCU syscon, the reset register
>> +			  offset and the status register offset.
>> +- #reset-cells		: Specifies the number of cells needed to encode the
>> +			  reset line, should be 1.
>> +
>> +Optional properties:
>> +- reset-status		: The request status bit. For some bits the request bit
>> +			  and the status bit are different. This is depending
>> +			  on the SoC. If the reset-status bit does not match
>> +			  the reset-request bit, put the reset number into the
>> +			  reset-request property and the status bit at the same
>> +			  index into the reset-status property. If no
>> +			  reset-request bit is given here, the driver assume
>> +			  status and request bit are the same.
>> +- reset-request		: The reset request bit, to map it to the reset-status
>> +			  bit.
> 
> These should either be implied by SoC specific compatible or be made 
> part of the reset cells. In the latter case, you still need the SoC 
> specific compatible.

Currently the reset framework only supports a single reset cell to my
knowledge, but I haven't looked into the details, I could extend it to
make it support two.

The SoC which needs this has two reset control register sets and the
bits are specific for each register set. Would a specific compatible
string for each register set ok?

> 
>> +-------------------------------------------------------------------------------
>> +Example for the reset-controllers on the xRX200 SoCs:
>> +	rcu_reset0: rcu_reset {
>> +		compatible = "lantiq,rcu-reset";
>> +		lantiq,rcu-syscon = <&rcu0 0x10 0x14>;
>> +		#reset-cells = <1>;
>> +		reset-request = <31>, <29>, <21>, <19>, <16>, <12>;
>> +		reset-status  = <30>, <28>, <16>, <25>, <5>,  <24>;
>> +	};
>> +
>> +	rcu_reset1: rcu_reset {
>> +		compatible = "lantiq,rcu-reset";
> 
> These 2 blocks are identical? Given different registers sizes, I'd say 
> not. So they should have different compatible strings.

I will remove the second one.

> 
>> +		lantiq,rcu-syscon = <&rcu0 0x48 0x24>;
>> +		#reset-cells = <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

^ permalink raw reply

* Re: [PATCH 06/13] MIPS: lantiq: Convert the xbar driver to a platform_driver
From: Hauke Mehrtens @ 2017-04-25  6:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: ralf-6z/3iImG2C8G8FEW9MqTrA, linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg,
	john-Pj+rj9U5foFAfugRpC6u6w, linux-spi-u79uwXL29TY76Z2rM5mHXA,
	hauke.mehrtens-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20170420144848.hwvtrhnwxcsxyv7x@rob-hp-laptop>



On 04/20/2017 04:48 PM, Rob Herring wrote:
> On Mon, Apr 17, 2017 at 09:29:35PM +0200, Hauke Mehrtens wrote:
>> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> This allows using the xbar driver on ARX300 based SoCs which require the
>> same xbar setup as the xRX200 chipsets because the xbar driver
>> initialization is not guarded by an xRX200 specific
>> of_machine_is_compatible condition anymore. Additionally the new driver
>> takes a syscon phandle to configure the XBAR endianness bits in RCU
>> (before this was done in arch/mips/lantiq/xway/reset.c and also
>> guarded by an xRX200 specific if-statement).
>>
>> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
>> ---
>>  .../devicetree/bindings/mips/lantiq/xbar.txt       |  22 +++++
>>  MAINTAINERS                                        |   1 +
>>  arch/mips/lantiq/xway/reset.c                      |   4 -
>>  arch/mips/lantiq/xway/sysctrl.c                    |  41 ---------
>>  drivers/soc/Makefile                               |   1 +
>>  drivers/soc/lantiq/Makefile                        |   1 +
>>  drivers/soc/lantiq/xbar.c                          | 100 +++++++++++++++++++++
>>  7 files changed, 125 insertions(+), 45 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mips/lantiq/xbar.txt
>>  create mode 100644 drivers/soc/lantiq/Makefile
>>  create mode 100644 drivers/soc/lantiq/xbar.c
>>
>> diff --git a/Documentation/devicetree/bindings/mips/lantiq/xbar.txt b/Documentation/devicetree/bindings/mips/lantiq/xbar.txt
>> new file mode 100644
>> index 000000000000..86e53ff3b0d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mips/lantiq/xbar.txt
>> @@ -0,0 +1,22 @@
>> +Lantiq XWAY SoC XBAR binding
>> +============================
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Required properties:
>> +- compatible	: Should be "lantiq,xbar-xway"
> 
> This compatible is already in use so it is fine, but you should also 
> have per SoC compatible strings.

I will add a new SoC specific one.
What does per SoC device tree mean? Does it mean for the same silicon,
for the same silicon revision, for the same fusing of a silicon or for
the same marketing name?

I would like to make it per silicon or per silicon revision for the IP
cores which I know are different.

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

^ permalink raw reply

* Re: [PATCH v3 1/3] mtd: nand: Cleanup/rework the atmel_nand driver
From: Boris Brezillon @ 2017-04-25  6:53 UTC (permalink / raw)
  To: Brian Norris
  Cc: Richard Weinberger, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Nicolas Ferre, Alexandre Belloni, Haavard Skinnemoen,
	Hans-Christian Egtvedt, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Wenyou Yang, Josh Wu, David Woodhouse, Marek Vasut,
	Cyrille Pitchen,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Masahiro Yamada
In-Reply-To: <20170425010930.GA120971-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Mon, 24 Apr 2017 18:09:30 -0700
Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Thu, Mar 16, 2017 at 09:02:40AM +0100, Boris Brezillon wrote:
> 
> > diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
> > new file mode 100644
> > index 000000000000..f71b9e5d7d9d
> > --- /dev/null
> > +++ b/drivers/mtd/nand/atmel/nand-controller.c
> > @@ -0,0 +1,2198 @@  
> [...]
> 
> > +static int
> > +atmel_hsmc_nand_controller_legacy_init(struct atmel_hsmc_nand_controller *nc)
> > +{
> > +	struct regmap_config regmap_conf = {
> > +		.reg_bits = 32,
> > +		.val_bits = 32,
> > +		.reg_stride = 4,
> > +		.val_bits = 32,  
> 
> You assigned val_bits twice. Is that just a harmless mistake, or did you
> mean to set something else?

Nope, I didn't mean to set another field to 32. I'll just apply the fix
you provided.

Thanks,

Boris
--
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

^ permalink raw reply

* Re: [PATCH 11/13] phy: Add an USB PHY driver for the Lantiq SoCs using the RCU module
From: Hauke Mehrtens @ 2017-04-25  6:51 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: ralf-6z/3iImG2C8G8FEW9MqTrA, linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, john-Pj+rj9U5foFAfugRpC6u6w,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	hauke.mehrtens-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <CAFBinCAB=vaDpzCoMFX8w9j0R04i6Zr4mbjDtteKsQ_LkKAaLg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 04/17/2017 11:23 PM, Martin Blumenstingl wrote:
> On Mon, Apr 17, 2017 at 9:29 PM, Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org> wrote:
>> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>
>> This driver starts the DWC2 core(s) built into the XWAY SoCs and provides
>> the PHY interfaces for each core. The phy instances can be passed to the
>> dwc2 driver, which already supports the generic phy interface.
>>
>> Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
> you should probably send this patch to the PHY maintainer as well
> (Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>)
> 
>> ---
>>  .../bindings/phy/phy-lantiq-rcu-usb2.txt           |  59 ++++
>>  arch/mips/lantiq/xway/reset.c                      |  43 ---
>>  arch/mips/lantiq/xway/sysctrl.c                    |  24 +-
>>  drivers/phy/Kconfig                                |   8 +
>>  drivers/phy/Makefile                               |   1 +
>>  drivers/phy/phy-lantiq-rcu-usb2.c                  | 325 +++++++++++++++++++++
>>  6 files changed, 405 insertions(+), 55 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt
>>  create mode 100644 drivers/phy/phy-lantiq-rcu-usb2.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt b/Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt
>> new file mode 100644
>> index 000000000000..0ec9f790b6e0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-lantiq-rcu-usb2.txt
>> @@ -0,0 +1,59 @@
>> +Lantiq XWAY SoC RCU USB 1.1/2.0 PHY binding
>> +===========================================
>> +
>> +This binding describes the USB PHY hardware provided by the RCU module on the
>> +Lantiq XWAY SoCs.
>> +
>> +
>> +-------------------------------------------------------------------------------
>> +Required properties (controller (parent) node):
>> +- compatible           : Should be one of
>> +                               "lantiq,ase-rcu-usb2-phy"
>> +                               "lantiq,danube-rcu-usb2-phy"
>> +                               "lantiq,xrx100-rcu-usb2-phy"
>> +                               "lantiq,xrx200-rcu-usb2-phy"
>> +                               "lantiq,xrx300-rcu-usb2-phy"
>> +- lantiq,rcu-syscon    : A phandle to the RCU module and the offsets to the
>> +                         USB PHY configuration and USB MAC registers.
>> +- address-cells                : should be 1
>> +- size-cells           : should be 0
>> +- phy-cells            : from the generic PHY bindings, must be 1
>> +
>> +Optional properties (controller (parent) node):
>> +- vbus-gpio            : References a GPIO which enables VBUS all given USB
>> +                         ports.
> the PHY framework already handles this if you wrap the GPIO in a
> "regulator-fixed" node, see [0] how to define a fixed regulator with a
> GPIO (the regulator in this example has two states: off = 0V and on =
> 5V, probably exactly what you need) and [1] how to pass it to the PHY
> (phy-core.c handles this already, no driver specific code needed)

Thanksy, I will change the code and use a regulator.

......

>> diff --git a/arch/mips/lantiq/xway/reset.c b/arch/mips/lantiq/xway/reset.c
>> index 3f30fb81a50f..5aec1f54275b 100644
>> --- a/arch/mips/lantiq/xway/reset.c
>> +++ b/arch/mips/lantiq/xway/reset.c
> could these arch/mips/lantiq/xway/reset.c changes to into PATCH #3 as well?

I do not get this.

......

> [0] https://github.com/torvalds/linux/blob/2fbbc4bf69f293df317559a267f4120f290b8fc4/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi#L67
> [1] https://github.com/torvalds/linux/blob/2fbbc4bf69f293df317559a267f4120f290b8fc4/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi#L133
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V2  2/2] clk: rockchip: fix the incorrect ids
From: Eddie Cai @ 2017-04-25  6:41 UTC (permalink / raw)
  To: robh+dt, mark.rutland, heiko, zhengxing, mturquette, sboyd
  Cc: devicetree, linux-kernel, linux-clk, linux-arm-kernel,
	linux-rockchip, Eddie Cai
In-Reply-To: <1493102470-22965-1-git-send-email-eddie.cai.linux@gmail.com>

the ids of clk_testout1 and clk_testout2 is incorrect now. let's correct it

Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
---
 drivers/clk/rockchip/clk-rk3399.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index 73121b14..f3656ba 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1066,13 +1066,13 @@ enum rk3399_pmu_plls {
 	/* cif_testout */
 	MUX(0, "clk_testout1_pll_src", mux_pll_src_cpll_gpll_npll_p, 0,
 			RK3399_CLKSEL_CON(38), 6, 2, MFLAGS),
-	COMPOSITE(0, "clk_testout1", mux_clk_testout1_p, 0,
+	COMPOSITE(SCLK_TESTCLKOUT1, "clk_testout1", mux_clk_testout1_p, 0,
 			RK3399_CLKSEL_CON(38), 5, 1, MFLAGS, 0, 5, DFLAGS,
 			RK3399_CLKGATE_CON(13), 14, GFLAGS),
 
 	MUX(0, "clk_testout2_pll_src", mux_pll_src_cpll_gpll_npll_p, 0,
 			RK3399_CLKSEL_CON(38), 14, 2, MFLAGS),
-	COMPOSITE(0, "clk_testout2", mux_clk_testout2_p, 0,
+	COMPOSITE(SCLK_TESTCLKOUT2, "clk_testout2", mux_clk_testout2_p, 0,
 			RK3399_CLKSEL_CON(38), 13, 1, MFLAGS, 8, 5, DFLAGS,
 			RK3399_CLKGATE_CON(13), 15, GFLAGS),
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH V2  1/2] clk: rockchip: add ids for camera
From: Eddie Cai @ 2017-04-25  6:41 UTC (permalink / raw)
  To: robh+dt, mark.rutland, heiko, zhengxing, mturquette, sboyd
  Cc: devicetree, linux-kernel, linux-clk, linux-arm-kernel,
	linux-rockchip, Eddie Cai
In-Reply-To: <1493102470-22965-1-git-send-email-eddie.cai.linux@gmail.com>

we use SCLK_TESTCLKOUT1 and SCLK_TESTCLKOUT2 for camera, so add those ids.

Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
---
 include/dt-bindings/clock/rk3399-cru.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/rk3399-cru.h b/include/dt-bindings/clock/rk3399-cru.h
index 220a60f..22cb1df 100644
--- a/include/dt-bindings/clock/rk3399-cru.h
+++ b/include/dt-bindings/clock/rk3399-cru.h
@@ -132,6 +132,8 @@
 #define SCLK_RMII_SRC			166
 #define SCLK_PCIEPHY_REF100M		167
 #define SCLK_DDRC			168
+#define SCLK_TESTCLKOUT1		169
+#define SCLK_TESTCLKOUT2		170
 
 #define DCLK_VOP0			180
 #define DCLK_VOP1			181
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2  0/2] correct ids for clk_testout
From: Eddie Cai @ 2017-04-25  6:41 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	heiko-4mtYJXux2i+zQB+pC5nmwQ, zhengxing-TNX95d0MmH7DzftRWevZcw,
	mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Eddie Cai

we use clk_testout1 and clk_testout2 for camera. e.g. mipi 24M mclk. the ids of
clk_testout1 and clk_testout2 is wrong. it's time to correct it. 

Eddie Cai (2):
  clk: rockchip: add ids for camera
  clk: rockchip: fix the incorrect ids

 drivers/clk/rockchip/clk-rk3399.c      | 4 ++--
 include/dt-bindings/clock/rk3399-cru.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
1.9.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

^ permalink raw reply

* Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)
From: Stefan Wahren @ 2017-04-25  6:22 UTC (permalink / raw)
  To: Eric Anholt, Lee Jones, Florian Fainelli, Olof Johansson,
	Rob Herring, Mark Rutland, devicetree
  Cc: linux-rpi-kernel, bcm-kernel-feedback-list, linux-kernel,
	linux-arm-kernel, Gerd Hoffmann
In-Reply-To: <20170424200037.25615-1-eric@anholt.net>

Am 24.04.2017 um 22:00 schrieb Eric Anholt:
> Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
> now, so it's useful to be able to test that mode on an upstream
> kernel.  It's also been useful for me to use the same board for 32-bit
> and 64-bit development.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  arch/arm/boot/dts/Makefile            | 1 +
>  arch/arm/boot/dts/bcm2837-rpi-3.b.dts | 1 +
>  2 files changed, 2 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm2837-rpi-3.b.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 011808490fed..eded842d9978 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>  	bcm2835-rpi-b-plus.dtb \
>  	bcm2835-rpi-a-plus.dtb \
>  	bcm2836-rpi-2-b.dtb \
> +	bcm2837-rpi-3-b.dtb \
>  	bcm2835-rpi-zero.dtb
>  dtb-$(CONFIG_ARCH_BCM_5301X) += \
>  	bcm4708-asus-rt-ac56u.dtb \
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3.b.dts b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> new file mode 100644
> index 000000000000..8c8aa4d1e9b3
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> @@ -0,0 +1 @@
> +#include "arm64/broadcom/bcm2837-rpi-3.b.dts"

Looks like a typo in the dts filename and in the include ( dot instead
of dash ).

^ permalink raw reply

* [PATCH v2 4/4] of: detect invalid phandle in overlay
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25  6:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493101210-11745-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Overlays are not allowed to modify phandle values of previously existing
nodes because there is no information available to allow fixup up
properties that use the previously existing phandle.

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/overlay.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index ca0b85f5deb1..20ab49d2f7a4 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -130,6 +130,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 	/* NOTE: Multiple mods of created nodes not supported */
 	tchild = of_get_child_by_name(target, cname);
 	if (tchild != NULL) {
+		/* new overlay phandle value conflicts with existing value */
+		if (child->phandle)
+			return -EINVAL;
+
 		/* apply overlay recursively */
 		ret = of_overlay_apply_one(ov, tchild, child);
 		of_node_put(tchild);
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

--
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

^ permalink raw reply related

* [PATCH v2 3/4] of: be consistent in form of file mode
From: frowand.list @ 2017-04-25  6:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel
In-Reply-To: <1493101210-11745-1-git-send-email-frowand.list@gmail.com>

From: Frank Rowand <frank.rowand@sony.com>

checkpatch whined about using S_IRUGO instead of octal equivalent
when adding phandle sysfs code, so used octal in that patch.
Change other instances of the S_* constants in the same file to
the octal form.

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 drivers/of/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9e70d283cdc5..865e54355733 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -168,7 +168,7 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
 
 	sysfs_bin_attr_init(&pp->attr);
 	pp->attr.attr.name = safe_name(&np->kobj, pp->name);
-	pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
+	pp->attr.attr.mode = secure ? 0400 : 0444;
 	pp->attr.size = secure ? 0 : pp->length;
 	pp->attr.read = of_node_property_read;
 
-- 
Frank Rowand <frank.rowand@sony.com>

^ permalink raw reply related

* [PATCH v2 2/4] of: make __of_attach_node() static
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25  6:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493101210-11745-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

__of_attach_node() is not used outside of drivers/of/dynamic.c.  Make
it static and remove it from drivers/of/of_private.h.

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/dynamic.c    | 2 +-
 drivers/of/of_private.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 59545b8fbf46..0b9cf6d5914c 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -216,7 +216,7 @@ int of_property_notify(int action, struct device_node *np,
 	return of_reconfig_notify(action, &pr);
 }
 
-void __of_attach_node(struct device_node *np)
+static void __of_attach_node(struct device_node *np)
 {
 	np->child = NULL;
 	np->sibling = np->parent->child;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 33f11a5384f3..b1ff70e1fdda 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -79,7 +79,6 @@ extern int __of_update_property(struct device_node *np,
 extern void __of_update_property_sysfs(struct device_node *np,
 		struct property *newprop, struct property *oldprop);
 
-extern void __of_attach_node(struct device_node *np);
 extern int __of_attach_node_sysfs(struct device_node *np);
 extern void __of_detach_node(struct device_node *np);
 extern void __of_detach_node_sysfs(struct device_node *np);
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

--
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

^ permalink raw reply related

* [PATCH v2 1/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25  6:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493101210-11745-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Remove "phandle", "linux,phandle", and "ibm,phandle" properties from
the internal device tree.  The phandle will still be in the struct
device_node phandle field.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *.  As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

  [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
  in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
  attached to the live tree.  Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
  __of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
  properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
  "ibm,phandle" properties will no longer appear in /proc/device-tree (they
  will appear as "phandle").

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/base.c       | 51 +++++++++++++++++++++++++++++++++++++++++++---
 drivers/of/dynamic.c    | 54 +++++++++++++++++++++++++++++++++++++------------
 drivers/of/fdt.c        | 40 +++++++++++++++++++++---------------
 drivers/of/of_private.h |  1 +
 drivers/of/overlay.c    |  4 +---
 drivers/of/resolver.c   | 23 +--------------------
 include/linux/of.h      |  1 +
 7 files changed, 117 insertions(+), 57 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2d..9e70d283cdc5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
 	return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
 }
 
+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+				struct bin_attribute *bin_attr, char *buf,
+				loff_t offset, size_t count)
+{
+	phandle phandle;
+	struct device_node *np;
+
+	np = container_of(bin_attr, struct device_node, attr_phandle);
+	phandle = cpu_to_be32(np->phandle);
+	return memory_read_from_buffer(buf, count, &offset, &phandle,
+				       sizeof(phandle));
+}
+
 /* always return newly allocated name, caller must free after use */
 static const char *safe_name(struct kobject *kobj, const char *orig_name)
 {
@@ -164,6 +177,38 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
 	return rc;
 }
 
+/*
+ * In the imported device tree (fdt), phandle is a property.  In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+	int rc;
+
+	if (IS_ENABLED(CONFIG_PPC_PSERIES))
+		return 0;
+
+	if (!IS_ENABLED(CONFIG_SYSFS))
+		return 0;
+
+	if (!of_kset || !of_node_is_attached(np))
+		return 0;
+
+	if (!np->phandle || np->phandle == 0xffffffff)
+		return 0;
+
+	sysfs_bin_attr_init(&pp->attr);
+	np->attr_phandle.attr.name = "phandle";
+	np->attr_phandle.attr.mode = 0444;
+	np->attr_phandle.size = sizeof(np->phandle);
+	np->attr_phandle.read = of_node_phandle_read;
+
+	rc = sysfs_create_bin_file(&np->kobj, &np->attr_phandle);
+	WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+	return rc;
+}
+
 int __of_attach_node_sysfs(struct device_node *np)
 {
 	const char *name;
@@ -193,6 +238,8 @@ int __of_attach_node_sysfs(struct device_node *np)
 	if (rc)
 		return rc;
 
+	__of_add_phandle_sysfs(np);
+
 	for_each_property_of_node(np, pp)
 		__of_add_property_sysfs(np, pp);
 
@@ -2097,9 +2144,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 		int id, len;
 
 		/* Skip those we do not want to proceed */
-		if (!strcmp(pp->name, "name") ||
-		    !strcmp(pp->name, "phandle") ||
-		    !strcmp(pp->name, "linux,phandle"))
+		if (!strcmp(pp->name, "name"))
 			continue;
 
 		np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..59545b8fbf46 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,
 
 void __of_attach_node(struct device_node *np)
 {
-	const __be32 *phandle;
-	int sz;
-
-	np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
-	np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
-
-	phandle = __of_get_property(np, "phandle", &sz);
-	if (!phandle)
-		phandle = __of_get_property(np, "linux,phandle", &sz);
-	if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
-		phandle = __of_get_property(np, "ibm,phandle", &sz);
-	np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
-
 	np->child = NULL;
 	np->sibling = np->parent->child;
 	np->parent->child = np;
@@ -243,11 +230,47 @@ void __of_attach_node(struct device_node *np)
 int of_attach_node(struct device_node *np)
 {
 	struct of_reconfig_data rd;
+	struct property *prev;
+	struct property *prop;
+	struct property *save_next;
 	unsigned long flags;
+	const __be32 *phandle;
+	int sz;
 
 	memset(&rd, 0, sizeof(rd));
 	rd.dn = np;
 
+	np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
+	np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
+
+	phandle = __of_get_property(np, "phandle", &sz);
+	if (!phandle)
+		phandle = __of_get_property(np, "linux,phandle", &sz);
+	if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
+		phandle = __of_get_property(np, "ibm,phandle", &sz);
+	np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
+
+	/* remove phandle properties from node */
+	prev = NULL;
+	for (prop = np->properties; prop != NULL; ) {
+		save_next = prop->next;
+		if (!strcmp(prop->name, "phandle") ||
+		    !strcmp(prop->name, "ibm,phandle") ||
+		    !strcmp(prop->name, "linux,phandle")) {
+			if (prev)
+				prev->next = prop->next;
+			else
+				np->properties = prop->next;
+			prop->next = np->deadprops;
+			np->deadprops = prop;
+		} else {
+			prev = prop;
+		}
+		prop = save_next;
+	}
+
+	__of_add_phandle_sysfs(np);
+
 	mutex_lock(&of_mutex);
 	raw_spin_lock_irqsave(&devtree_lock, flags);
 	__of_attach_node(np);
@@ -429,6 +452,7 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
 	/* Iterate over and duplicate all properties */
 	if (np) {
 		struct property *pp, *new_pp;
+		node->phandle = np->phandle;
 		for_each_property_of_node(np, pp) {
 			new_pp = __of_prop_dup(pp, GFP_KERNEL);
 			if (!new_pp)
@@ -441,6 +465,10 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
 			}
 		}
 	}
+
+	node->name = __of_get_property(node, "name", NULL) ? : "<NULL>";
+	node->type = __of_get_property(node, "device_type", NULL) ? : "<NULL>";
+
 	return node;
 
  err_prop:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..270f31b0c259 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -181,6 +181,8 @@ static void populate_properties(const void *blob,
 		const __be32 *val;
 		const char *pname;
 		u32 sz;
+		int prop_is_phandle = 0;
+		int prop_is_ibm_phandle = 0;
 
 		val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
 		if (!val) {
@@ -196,11 +198,6 @@ static void populate_properties(const void *blob,
 		if (!strcmp(pname, "name"))
 			has_name = true;
 
-		pp = unflatten_dt_alloc(mem, sizeof(struct property),
-					__alignof__(struct property));
-		if (dryrun)
-			continue;
-
 		/* We accept flattened tree phandles either in
 		 * ePAPR-style "phandle" properties, or the
 		 * legacy "linux,phandle" properties.  If both
@@ -208,23 +205,34 @@ static void populate_properties(const void *blob,
 		 * will get weird. Don't do that.
 		 */
 		if (!strcmp(pname, "phandle") ||
-		    !strcmp(pname, "linux,phandle")) {
-			if (!np->phandle)
-				np->phandle = be32_to_cpup(val);
-		}
+		    !strcmp(pname, "linux,phandle"))
+			prop_is_phandle = 1;
 
 		/* And we process the "ibm,phandle" property
 		 * used in pSeries dynamic device tree
 		 * stuff
 		 */
-		if (!strcmp(pname, "ibm,phandle"))
-			np->phandle = be32_to_cpup(val);
+		if (!strcmp(pname, "ibm,phandle")) {
+			prop_is_phandle = 1;
+			prop_is_ibm_phandle = 1;
+		}
+
+		if (!prop_is_phandle)
+			pp = unflatten_dt_alloc(mem, sizeof(struct property),
+						__alignof__(struct property));
 
-		pp->name   = (char *)pname;
-		pp->length = sz;
-		pp->value  = (__be32 *)val;
-		*pprev     = pp;
-		pprev      = &pp->next;
+		if (dryrun)
+			continue;
+
+		if (!prop_is_phandle) {
+			pp->name   = (char *)pname;
+			pp->length = sz;
+			pp->value  = (__be32 *)val;
+			*pprev     = pp;
+			pprev      = &pp->next;
+		} else if (prop_is_ibm_phandle || !np->phandle) {
+			np->phandle = be32_to_cpup(val);
+		}
 	}
 
 	/* With version 0x10 we may not have the name property,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..33f11a5384f3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -47,6 +47,7 @@ extern int of_property_notify(int action, struct device_node *np,
 extern void of_node_release(struct kobject *kobj);
 extern int __of_changeset_apply(struct of_changeset *ocs);
 extern int __of_changeset_revert(struct of_changeset *ocs);
+extern int __of_add_phandle_sysfs(struct device_node *np);
 #else /* CONFIG_OF_DYNAMIC */
 static inline int of_property_notify(int action, struct device_node *np,
 				     struct property *prop, struct property *old_prop)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7827786718d8..ca0b85f5deb1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -101,9 +101,7 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
 	tprop = of_find_property(target, prop->name, NULL);
 
 	/* special properties are not meant to be updated (silent NOP) */
-	if (of_prop_cmp(prop->name, "name") == 0 ||
-	    of_prop_cmp(prop->name, "phandle") == 0 ||
-	    of_prop_cmp(prop->name, "linux,phandle") == 0)
+	if (!of_prop_cmp(prop->name, "name"))
 		return 0;
 
 	propn = __of_prop_dup(prop, GFP_KERNEL);
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 7ae9863cb0a4..624cbaeae0a4 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -71,30 +71,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
 		int phandle_delta)
 {
 	struct device_node *child;
-	struct property *prop;
-	phandle phandle;
 
 	/* adjust node's phandle in node */
 	if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
 		overlay->phandle += phandle_delta;
 
-	/* copy adjusted phandle into *phandle properties */
-	for_each_property_of_node(overlay, prop) {
-
-		if (of_prop_cmp(prop->name, "phandle") &&
-		    of_prop_cmp(prop->name, "linux,phandle"))
-			continue;
-
-		if (prop->length < 4)
-			continue;
-
-		phandle = be32_to_cpup(prop->value);
-		if (phandle == OF_PHANDLE_ILLEGAL)
-			continue;
-
-		*(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
-	}
-
 	for_each_child_of_node(overlay, child)
 		adjust_overlay_phandles(child, phandle_delta);
 }
@@ -197,9 +178,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
 	for_each_property_of_node(local_fixups, prop_fix) {
 
 		/* skip properties added automatically */
-		if (!of_prop_cmp(prop_fix->name, "name") ||
-		    !of_prop_cmp(prop_fix->name, "phandle") ||
-		    !of_prop_cmp(prop_fix->name, "linux,phandle"))
+		if (!of_prop_cmp(prop_fix->name, "name"))
 			continue;
 
 		if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323de0f3..4e86e05853f3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -61,6 +61,7 @@ struct device_node {
 	struct	kobject kobj;
 	unsigned long _flags;
 	void	*data;
+	struct bin_attribute attr_phandle;
 #if defined(CONFIG_SPARC)
 	const char *path_component_name;
 	unsigned int unique_id;
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

--
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

^ permalink raw reply related

* [PATCH v2 0/4] of: remove *phandle properties from expanded device tree
From: frowand.list @ 2017-04-25  6:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel

From: Frank Rowand <frank.rowand@sony.com>

Remove "phandle" and "linux,phandle" properties from the internal
device tree.  The phandle will still be in the struct device_node
phandle field.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *.  As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

  [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

Patch 1 is the phandle related changes.

Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.

Changes from v1:
   - Remove phandle properties in of_attach_node(), before attaching
     the node to the live tree.
   - Add the phandle sysfs entry for the node in of_attach_node().
   - When creating an overlay changeset, duplicate the node phandle in
     __of_node_dup().

Frank Rowand (4):
  of: remove *phandle properties from expanded device tree
  of: make __of_attach_node() static
  of: be consistent in form of file mode
  of: detect invalid phandle in overlay

 drivers/of/base.c       | 53 ++++++++++++++++++++++++++++++++++++++++++----
 drivers/of/dynamic.c    | 56 ++++++++++++++++++++++++++++++++++++-------------
 drivers/of/fdt.c        | 40 +++++++++++++++++++++--------------
 drivers/of/of_private.h |  2 +-
 drivers/of/overlay.c    |  8 ++++---
 drivers/of/resolver.c   | 23 +-------------------
 include/linux/of.h      |  1 +
 7 files changed, 123 insertions(+), 60 deletions(-)

-- 
Frank Rowand <frank.rowand@sony.com>

^ permalink raw reply

* Re: [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803
From: icenowy-h8G6r0blFSE @ 2017-04-25  5:47 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Thomas Gleixner, Rob Herring, Maxime Ripard, Lee Jones,
	Liam Girdwood, linux-kernel, devicetree, linux-arm-kernel,
	linux-sunxi
In-Reply-To: <CAGb2v673CL6TaZxkNR9SZDiG5G3cMqU1-=XeMj5gyDbQq4YzVA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

在 2017-04-25 10:17,Chen-Yu Tsai 写道:
> On Tue, Apr 25, 2017 at 12:01 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> 
> wrote:
>> As axp20x-regulator now supports AXP803, add a cell for it.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> Changes in v4:
>> - Added a trailing comma for new cell, for easier further cell 
>> addition.
>> Changes in v3:
>> - Make the new cell one-liner.
>> 
>>  drivers/mfd/axp20x.c                 | 3 ++-
>>  drivers/regulator/axp20x-regulator.c | 6 +++---
> 
> Squashed in wrong patch?

Oh seems so...

> 
> ChenYu

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)
From: Florian Fainelli @ 2017-04-25  4:14 UTC (permalink / raw)
  To: Eric Anholt, Lee Jones, Florian Fainelli, Olof Johansson,
	Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Gerd Hoffmann
In-Reply-To: <20170424200037.25615-1-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>



On 04/24/2017 01:00 PM, Eric Anholt wrote:
> Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
> now, so it's useful to be able to test that mode on an upstream
> kernel.  It's also been useful for me to use the same board for 32-bit
> and 64-bit development.
> 
> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile            | 1 +
>  arch/arm/boot/dts/bcm2837-rpi-3.b.dts | 1 +
>  2 files changed, 2 insertions(+)
>  create mode 100644 arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 011808490fed..eded842d9978 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
>  	bcm2835-rpi-b-plus.dtb \
>  	bcm2835-rpi-a-plus.dtb \
>  	bcm2836-rpi-2-b.dtb \
> +	bcm2837-rpi-3-b.dtb \
>  	bcm2835-rpi-zero.dtb
>  dtb-$(CONFIG_ARCH_BCM_5301X) += \
>  	bcm4708-asus-rt-ac56u.dtb \
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3.b.dts b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> new file mode 100644
> index 000000000000..8c8aa4d1e9b3
> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
> @@ -0,0 +1 @@
> +#include "arm64/broadcom/bcm2837-rpi-3.b.dts"

Sounds like this should be
../arm64/boot/dts/broadcom/bcm2837-rpi-3.b.dts are reported by the
kbuild test robot?
-- 
Florian
--
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

^ permalink raw reply

* Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)
From: kbuild test robot @ 2017-04-25  3:56 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, Lee Jones, Florian Fainelli,
	Olof Johansson, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Gerd Hoffmann,
	Eric Anholt
In-Reply-To: <20170424200037.25615-1-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>

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

Hi Eric,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.11-rc8 next-20170424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Eric-Anholt/ARM-dts-Add-devicetree-for-the-Raspberry-Pi-3-for-arm32-v5/20170425-063618
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> arch/arm/boot/dts/bcm2837-rpi-3.b.dts:1:46: fatal error: arm64/broadcom/bcm2837-rpi-3.b.dts: No such file or directory
    #include "arm64/broadcom/bcm2837-rpi-3.b.dts"
                                                 ^
   compilation terminated.

vim +1 arch/arm/boot/dts/bcm2837-rpi-3.b.dts

   > 1	#include "arm64/broadcom/bcm2837-rpi-3.b.dts"

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22403 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/2] Input: add support for the STMicroelectronics FingerTip touchscreen
From: Andi Shyti @ 2017-04-25  2:31 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring
  Cc: Javier Martinez Canillas, Andrzej Hajda, Chanwoo Choi,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti
In-Reply-To: <20170327130743.27783-3-andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Hi Dmitry,

again, kindly ping. This patch has been posted on March 27th and
this is the third time I ask you a feedback about it.

Please let me know,
Andi

On Mon, Mar 27, 2017 at 10:07:43PM +0900, Andi Shyti wrote:
> The stmfts (ST-Microelectronics FingerTip S) touchscreen device
> is a capacitive multi-touch controller mainly for mobile use.
> 
> It's connected through i2c bus at the address 0x49 and it
> interfaces with userspace through input event interface.
> 
> At the current state it provides a touchscreen multitouch
> functionality up to 10 fingers. Each finger is enumerated with a
> distinctive id (from 0 to 9).
> 
> If enabled the device can support single "touch" hovering, by
> providing three coordinates, x, y and distance.
> 
> It is possible to select the touchkey functionality which
> provides a basic two keys interface for "home" and "back" menu,
> typical in mobile phones.
> 
> Signed-off-by: Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/input/touchscreen/Kconfig  |  12 +
>  drivers/input/touchscreen/Makefile |   1 +
>  drivers/input/touchscreen/stmfts.c | 805 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 818 insertions(+)
>  create mode 100644 drivers/input/touchscreen/stmfts.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 33c62e5de4fa..f8631c64290d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1114,6 +1114,18 @@ config TOUCHSCREEN_ST1232
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called st1232_ts.
>  
> +config TOUCHSCREEN_STMFTS
> +	tristate "STMicroelectronics STMFTS touchscreen"
> +	depends on I2C
> +	depends on INPUT
> +	depends on LEDS_CLASS
> +	help
> +	  Say Y here if you want support for STMicroelectronics
> +	  STMFTS touchscreen.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called stmfts.
> +
>  config TOUCHSCREEN_STMPE
>  	tristate "STMicroelectronics STMPE touchscreens"
>  	depends on MFD_STMPE
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 18e476948e44..6badce87037b 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_TOUCHSCREEN_S3C2410)	+= s3c2410_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SILEAD)	+= silead.o
>  obj-$(CONFIG_TOUCHSCREEN_SIS_I2C)	+= sis_i2c.o
>  obj-$(CONFIG_TOUCHSCREEN_ST1232)	+= st1232.o
> +obj-$(CONFIG_TOUCHSCREEN_STMFTS)	+= stmfts.o
>  obj-$(CONFIG_TOUCHSCREEN_STMPE)		+= stmpe-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUN4I)		+= sun4i-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_SUR40)		+= sur40.o
> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> new file mode 100644
> index 000000000000..2e18b1456f42
> --- /dev/null
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -0,0 +1,805 @@
> +/*
> + * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> + * Author: Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * STMicroelectronics FTS Touchscreen device driver
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +
> +/* I2C commands */
> +#define STMFTS_READ_INFO			0x80
> +#define STMFTS_READ_STATUS			0x84
> +#define STMFTS_READ_ONE_EVENT			0x85
> +#define STMFTS_READ_ALL_EVENT			0x86
> +#define STMFTS_LATEST_EVENT			0x87
> +#define STMFTS_SLEEP_IN				0x90
> +#define STMFTS_SLEEP_OUT			0x91
> +#define STMFTS_MS_MT_SENSE_OFF			0x92
> +#define STMFTS_MS_MT_SENSE_ON			0x93
> +#define STMFTS_SS_HOVER_SENSE_OFF		0x94
> +#define STMFTS_SS_HOVER_SENSE_ON		0x95
> +#define STMFTS_MS_KEY_SENSE_OFF			0x9a
> +#define STMFTS_MS_KEY_SENSE_ON			0x9b
> +#define STMFTS_SYSTEM_RESET			0xa0
> +#define STMFTS_CLEAR_EVENT_STACK		0xa1
> +#define STMFTS_FULL_FORCE_CALIBRATION		0xa2
> +#define STMFTS_MS_CX_TUNING			0xa3
> +#define STMFTS_SS_CX_TUNING			0xa4
> +
> +/* events */
> +#define STMFTS_EV_NO_EVENT			0x00
> +#define STMFTS_EV_MULTI_TOUCH_DETECTED		0x02
> +#define STMFTS_EV_MULTI_TOUCH_ENTER		0x03
> +#define STMFTS_EV_MULTI_TOUCH_LEAVE		0x04
> +#define STMFTS_EV_MULTI_TOUCH_MOTION		0x05
> +#define STMFTS_EV_HOVER_ENTER			0x07
> +#define STMFTS_EV_HOVER_LEAVE			0x08
> +#define STMFTS_EV_HOVER_MOTION			0x09
> +#define STMFTS_EV_KEY_STATUS			0x0e
> +#define STMFTS_EV_ERROR				0x0f
> +#define STMFTS_EV_CONTROLLER_READY		0x10
> +#define STMFTS_EV_SLEEP_OUT_CONTROLLER_READY	0x11
> +#define STMFTS_EV_STATUS			0x16
> +#define STMFTS_EV_DEBUG				0xdb
> +
> +/* multi touch related event masks */
> +#define STMFTS_MASK_EVENT_ID			0x0f
> +#define STMFTS_MASK_TOUCH_ID			0xf0
> +#define STMFTS_MASK_LEFT_EVENT			0x0f
> +#define STMFTS_MASK_X_MSB			0x0f
> +#define STMFTS_MASK_Y_LSB			0xf0
> +
> +/* key related event masks */
> +#define STMFTS_MASK_KEY_NO_TOUCH		0x00
> +#define STMFTS_MASK_KEY_MENU			0x01
> +#define STMFTS_MASK_KEY_BACK			0x02
> +
> +#define STMFTS_EVENT_SIZE	8
> +#define STMFTS_STACK_DEPTH	32
> +#define STMFTS_DATA_MAX_SIZE	(STMFTS_EVENT_SIZE * STMFTS_STACK_DEPTH)
> +#define STMFTS_MAX_FINGERS	10
> +#define STMFTS_DEV_NAME		"stmfts"
> +
> +enum stmfts_regulators {
> +	STMFTS_REGULATOR_VDD,
> +	STMFTS_REGULATOR_AVDD,
> +};
> +
> +struct stmfts_data {
> +	struct i2c_client *client;
> +	struct input_dev *input;
> +	struct led_classdev led_cdev;
> +	struct mutex mutex;
> +
> +	struct touchscreen_properties prop;
> +
> +	struct regulator_bulk_data regulators[2];
> +
> +	/* ledvdd will be used also to check
> +	 * whether the LED is supported
> +	 */
> +	struct regulator *ledvdd;
> +
> +	u16 chip_id;
> +	u8 chip_ver;
> +	u16 fw_ver;
> +	u8 config_id;
> +	u8 config_ver;
> +
> +	u8 data[STMFTS_DATA_MAX_SIZE];
> +
> +	struct completion signal;
> +
> +	bool use_key;
> +	bool led_status;
> +	bool hover_enabled;
> +	bool running;
> +};
> +
> +static int stmfts_read_i2c_block_data(struct stmfts_data *sdata)
> +{
> +	struct i2c_msg msgs[2];
> +	u8 cmd = STMFTS_READ_ALL_EVENT;
> +
> +	msgs[0].addr = sdata->client->addr;
> +	msgs[0].flags = 0;
> +	msgs[0].len = 1;
> +	msgs[0].buf = &cmd;
> +
> +	msgs[1].addr = sdata->client->addr;
> +	msgs[1].flags = I2C_M_RD;
> +	msgs[1].len = STMFTS_DATA_MAX_SIZE - STMFTS_EVENT_SIZE;
> +	msgs[1].buf = sdata->data + STMFTS_EVENT_SIZE;
> +
> +	return i2c_transfer(sdata->client->adapter, msgs, ARRAY_SIZE(msgs));
> +}
> +
> +static void stmfts_brightness_set(struct led_classdev *led_cdev,
> +					enum led_brightness value)
> +{
> +	struct stmfts_data *sdata = container_of(led_cdev,
> +					struct stmfts_data, led_cdev);
> +
> +	if (value == sdata->led_status || !sdata->ledvdd)
> +		return;
> +
> +	if (!value) {
> +		regulator_disable(sdata->ledvdd);
> +	} else {
> +		int err = regulator_enable(sdata->ledvdd);
> +
> +		if (err)
> +			dev_warn(&sdata->client->dev,
> +				"failed to disable ledvdd regulator\n");
> +	}
> +
> +	sdata->led_status = value;
> +}
> +
> +static enum led_brightness stmfts_brightness_get(struct led_classdev *led_cdev)
> +{
> +	struct stmfts_data *sdata = container_of(led_cdev,
> +						struct stmfts_data, led_cdev);
> +
> +	return !!regulator_is_enabled(sdata->ledvdd);
> +}
> +
> +static void stmfts_parse_event(struct stmfts_data *sdata)
> +{
> +	u8 id, t_id;
> +	u16 x, y, z, maj, min, orientation, area;
> +	u8 *event;
> +	int i;
> +
> +	for (i = 0; i < STMFTS_STACK_DEPTH; i++) {
> +		event = &sdata->data[i*STMFTS_EVENT_SIZE];
> +
> +		id = event[0] & STMFTS_MASK_EVENT_ID;
> +		t_id = (event[0] & STMFTS_MASK_TOUCH_ID) >> 4;
> +
> +		switch (id) {
> +		case STMFTS_EV_NO_EVENT:
> +			return;
> +
> +		case STMFTS_EV_MULTI_TOUCH_ENTER:
> +		case STMFTS_EV_MULTI_TOUCH_LEAVE:
> +		case STMFTS_EV_MULTI_TOUCH_MOTION:
> +			if (id == STMFTS_EV_MULTI_TOUCH_ENTER)
> +				input_mt_report_slot_state(sdata->input,
> +							MT_TOOL_FINGER, true);
> +			else if (id == STMFTS_EV_MULTI_TOUCH_LEAVE)
> +				input_mt_report_slot_state(sdata->input,
> +							MT_TOOL_FINGER, false);
> +
> +			x = event[1] | ((event[2] & STMFTS_MASK_X_MSB) << 8);
> +			y = (event[2] >> 4) | (event[3] << 4);
> +
> +			maj = event[4];
> +			min = event[5];
> +			orientation = event[6];
> +			area = event[7];
> +
> +			input_mt_slot(sdata->input, t_id);
> +			input_report_abs(sdata->input, ABS_MT_POSITION_X, x);
> +			input_report_abs(sdata->input, ABS_MT_POSITION_Y, y);
> +			input_report_abs(sdata->input, ABS_MT_TOUCH_MAJOR, maj);
> +			input_report_abs(sdata->input, ABS_MT_TOUCH_MINOR, min);
> +			input_report_abs(sdata->input, ABS_MT_PRESSURE, area);
> +			input_report_abs(sdata->input, ABS_MT_ORIENTATION,
> +								orientation);
> +			input_sync(sdata->input);
> +
> +			break;
> +
> +		case STMFTS_EV_HOVER_ENTER:
> +		case STMFTS_EV_HOVER_LEAVE:
> +		case STMFTS_EV_HOVER_MOTION:
> +			x = (event[2] << 4) | (event[4] >> 4);
> +			y = (event[3] << 4) | (event[4] & STMFTS_MASK_Y_LSB);
> +			z = event[5];
> +			orientation = event[6] & STMFTS_MASK_Y_LSB;
> +
> +			input_report_abs(sdata->input, ABS_X, x);
> +			input_report_abs(sdata->input, ABS_Y, y);
> +			input_report_abs(sdata->input, ABS_DISTANCE, z);
> +			input_sync(sdata->input);
> +
> +			break;
> +
> +		case STMFTS_EV_KEY_STATUS:
> +			switch (event[2]) {
> +			case 0:
> +				input_report_key(sdata->input, KEY_BACK, 0);
> +				input_report_key(sdata->input, KEY_MENU, 0);
> +				break;
> +
> +			case STMFTS_MASK_KEY_BACK:
> +				input_report_key(sdata->input, KEY_BACK, 1);
> +				break;
> +
> +			case STMFTS_MASK_KEY_MENU:
> +				input_report_key(sdata->input, KEY_MENU, 1);
> +				break;
> +
> +			default:
> +				dev_warn(&sdata->client->dev,
> +						"unknown key event\n");
> +			}
> +
> +			input_sync(sdata->input);
> +			break;
> +
> +		case STMFTS_EV_ERROR:
> +			dev_warn(&sdata->client->dev,
> +					"error code: 0x%x%x%x%x%x%x",
> +					event[6], event[5], event[4],
> +					event[3], event[2], event[1]);
> +			break;
> +
> +		default:
> +			dev_err(&sdata->client->dev,
> +				"unknown event 0x%x\n", event[0]);
> +		}
> +	}
> +}
> +
> +static irqreturn_t stmfts_irq_handler(int irq, void *dev)
> +{
> +	struct stmfts_data *sdata = dev;
> +	int ret;
> +
> +	mutex_lock(&sdata->mutex);
> +	ret = i2c_smbus_read_i2c_block_data(sdata->client,
> +						STMFTS_READ_ONE_EVENT,
> +						STMFTS_EVENT_SIZE, sdata->data);
> +
> +	if (ret < 0 || ret != STMFTS_EVENT_SIZE)
> +		goto exit;
> +
> +	switch (sdata->data[0]) {
> +	case STMFTS_EV_CONTROLLER_READY:
> +	case STMFTS_EV_SLEEP_OUT_CONTROLLER_READY:
> +	case STMFTS_EV_STATUS:
> +		complete(&sdata->signal);
> +	case STMFTS_EV_NO_EVENT:
> +	case STMFTS_EV_DEBUG:
> +		break;
> +
> +	default:
> +		if (unlikely(!sdata->input))
> +			goto exit;
> +
> +		ret = stmfts_read_i2c_block_data(sdata);
> +		if (ret < 0)
> +			goto exit;
> +
> +		stmfts_parse_event(sdata);
> +	}
> +
> +exit:
> +	mutex_unlock(&sdata->mutex);
> +	return IRQ_HANDLED;
> +}
> +
> +static int stmfts_write_and_wait(struct stmfts_data *sdata, const u8 cmd)
> +{
> +	int err;
> +
> +	err = i2c_smbus_write_byte(sdata->client, cmd);
> +	if (err)
> +		return err;
> +
> +	err = wait_for_completion_timeout(&sdata->signal,
> +					msecs_to_jiffies(1000));
> +
> +	return !err ? -ETIMEDOUT : 0;
> +}
> +
> +static int stmfts_input_open(struct input_dev *dev)
> +{
> +	int ret;
> +	struct stmfts_data *sdata = input_get_drvdata(dev);
> +
> +	ret = pm_runtime_get_sync(&sdata->client->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_smbus_write_byte(sdata->client, STMFTS_MS_MT_SENSE_ON);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&sdata->mutex);
> +	sdata->running = true;
> +
> +	if (sdata->hover_enabled) {
> +		ret = i2c_smbus_write_byte(sdata->client,
> +						STMFTS_SS_HOVER_SENSE_ON);
> +		if (ret)
> +			dev_warn(&sdata->client->dev,
> +						"failed to enable hover\n");
> +	}
> +	mutex_unlock(&sdata->mutex);
> +
> +	if (sdata->use_key) {
> +		ret = i2c_smbus_write_byte(sdata->client,
> +						STMFTS_MS_KEY_SENSE_ON);
> +		if (ret)
> +			/* I can still use only the touch screen */
> +			dev_warn(&sdata->client->dev,
> +						"failed to enable touchkey\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static void stmfts_input_close(struct input_dev *dev)
> +{
> +	int ret;
> +	struct stmfts_data *sdata = input_get_drvdata(dev);
> +
> +	ret = i2c_smbus_write_byte(sdata->client, STMFTS_MS_MT_SENSE_OFF);
> +	if (ret)
> +		dev_warn(&sdata->client->dev,
> +					"failed to disable touchscreen\n");
> +
> +	mutex_lock(&sdata->mutex);
> +	sdata->running = false;
> +
> +	if (sdata->hover_enabled) {
> +		ret = i2c_smbus_write_byte(sdata->client,
> +					STMFTS_SS_HOVER_SENSE_OFF);
> +		if (ret)
> +			dev_warn(&sdata->client->dev,
> +						"failed to disable hover\n");
> +	}
> +	mutex_unlock(&sdata->mutex);
> +
> +	if (sdata->use_key) {
> +		i2c_smbus_write_byte(sdata->client, STMFTS_MS_KEY_SENSE_OFF);
> +		if (ret)
> +			dev_warn(&sdata->client->dev,
> +					"failed to disable touchkey\n");
> +	}
> +
> +	pm_runtime_put_sync(&sdata->client->dev);
> +}
> +
> +static ssize_t stmfts_sysfs_chip_id(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "0x%x\n", sdata->chip_id);
> +}
> +
> +static ssize_t stmfts_sysfs_chip_version(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", sdata->chip_ver);
> +}
> +
> +static ssize_t stmfts_sysfs_fw_ver(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", sdata->fw_ver);
> +}
> +
> +static ssize_t stmfts_sysfs_config_id(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "0x%x\n", sdata->config_id);
> +}
> +
> +static ssize_t stmfts_sysfs_config_version(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", sdata->config_ver);
> +}
> +
> +static ssize_t stmfts_sysfs_read_status(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +	u8 status[4];
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(sdata->client,
> +					STMFTS_READ_STATUS, 4, status);
> +
> +	return sprintf(buf, "0x%x\n", status[0]);
> +}
> +
> +static ssize_t stmfts_sysfs_hover_enable_read(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", sdata->hover_enabled);
> +}
> +
> +static ssize_t stmfts_sysfs_hover_enable_write(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t len)
> +{
> +	unsigned long value;
> +	int err;
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	if (kstrtoul(buf, 0, &value))
> +		return -EINVAL;
> +
> +	mutex_lock(&sdata->mutex);
> +
> +	if (value & sdata->hover_enabled)
> +		goto out;
> +
> +	if (!sdata->running) {
> +		sdata->hover_enabled = !!value;
> +		goto out;
> +	}
> +
> +	if (value) {
> +		err = i2c_smbus_write_byte(sdata->client,
> +						STMFTS_SS_HOVER_SENSE_ON);
> +		sdata->hover_enabled = !err;
> +	} else {
> +		err = i2c_smbus_write_byte(sdata->client,
> +					STMFTS_SS_HOVER_SENSE_OFF);
> +		sdata->hover_enabled = !!err;
> +	}
> +
> +	if (err)
> +		dev_warn(&sdata->client->dev, "failed to %s hover\n",
> +						value ? "enable" : "disable");
> +out:
> +	mutex_unlock(&sdata->mutex);
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR(chip_id, 0444, stmfts_sysfs_chip_id, NULL);
> +static DEVICE_ATTR(chip_version, 0444, stmfts_sysfs_chip_version, NULL);
> +static DEVICE_ATTR(fw_ver, 0444, stmfts_sysfs_fw_ver, NULL);
> +static DEVICE_ATTR(config_id, 0444, stmfts_sysfs_config_id, NULL);
> +static DEVICE_ATTR(config_version, 0444, stmfts_sysfs_config_version, NULL);
> +static DEVICE_ATTR(status, 0444, stmfts_sysfs_read_status, NULL);
> +static DEVICE_ATTR(hover_enable, 0644, stmfts_sysfs_hover_enable_read,
> +					stmfts_sysfs_hover_enable_write);
> +
> +static struct attribute *stmfts_sysfs_attrs[] = {
> +	&dev_attr_chip_id.attr,
> +	&dev_attr_chip_version.attr,
> +	&dev_attr_fw_ver.attr,
> +	&dev_attr_config_id.attr,
> +	&dev_attr_config_version.attr,
> +	&dev_attr_status.attr,
> +	&dev_attr_hover_enable.attr,
> +	NULL
> +};
> +
> +static struct attribute_group stmfts_attribute_group = {
> +	.attrs = stmfts_sysfs_attrs
> +};
> +
> +static int stmfts_power_on(struct stmfts_data *sdata)
> +{
> +	int err;
> +	u8 reg[8];
> +
> +	err = regulator_bulk_enable(ARRAY_SIZE(sdata->regulators),
> +							sdata->regulators);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * the datasheet does not specify the power on time, but considering
> +	 * that the reset time is < 10ms, I sleep 20ms to be sure
> +	 */
> +	msleep(20);
> +
> +	err = i2c_smbus_read_i2c_block_data(sdata->client,
> +					STMFTS_READ_INFO, 8, reg);
> +	if (err < 0)
> +		return err;
> +	if (err != 8)
> +		return -EIO;
> +
> +	sdata->chip_id = (reg[6] << 8) | reg[7];
> +	sdata->chip_ver = reg[0];
> +	sdata->fw_ver = (reg[2] << 8) | reg[3];
> +	sdata->config_id = reg[4];
> +	sdata->config_ver = reg[5];
> +
> +	reinit_completion(&sdata->signal);
> +
> +	enable_irq(sdata->client->irq);
> +	err = stmfts_write_and_wait(sdata, STMFTS_SYSTEM_RESET);
> +	if (err)
> +		return err;
> +
> +	err = stmfts_write_and_wait(sdata, STMFTS_SLEEP_OUT);
> +	if (err)
> +		return err;
> +
> +	/* optional tuning */
> +	err = stmfts_write_and_wait(sdata, STMFTS_MS_CX_TUNING);
> +	if (err)
> +		dev_warn(&sdata->client->dev, "failed to perform mutual auto tune\n");
> +
> +	/* optional tuning */
> +	err = stmfts_write_and_wait(sdata, STMFTS_SS_CX_TUNING);
> +	if (err)
> +		dev_warn(&sdata->client->dev, "failed to perform self auto tune\n");
> +
> +	err = stmfts_write_and_wait(sdata, STMFTS_FULL_FORCE_CALIBRATION);
> +	if (err)
> +		return err;
> +
> +	/* at this point no one is using the touchscreen
> +	 * and I don't really care about the return value
> +	 */
> +	i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);
> +
> +	return 0;
> +}
> +
> +static void stmfts_power_off(void *data)
> +{
> +	struct stmfts_data *sdata = data;
> +
> +	disable_irq(sdata->client->irq);
> +	regulator_bulk_disable(ARRAY_SIZE(sdata->regulators),
> +						sdata->regulators);
> +}
> +
> +/* This function is void because I don't want to prevent using the touch key
> + * only because the LEDs don't get registered
> + */
> +static int stmfts_enable_led(struct stmfts_data *sdata)
> +{
> +	int err;
> +
> +	/* get the regulator for powering the leds on */
> +	sdata->ledvdd = devm_regulator_get(&sdata->client->dev, "ledvdd");
> +	if (IS_ERR(sdata->ledvdd))
> +		return PTR_ERR(sdata->ledvdd);
> +
> +	sdata->led_cdev.name = STMFTS_DEV_NAME;
> +	sdata->led_cdev.max_brightness = LED_ON;
> +	sdata->led_cdev.brightness = LED_OFF;
> +	sdata->led_cdev.brightness_set = stmfts_brightness_set;
> +	sdata->led_cdev.brightness_get = stmfts_brightness_get;
> +
> +	err = devm_led_classdev_register(&sdata->client->dev, &sdata->led_cdev);
> +	if (err) {
> +		devm_regulator_put(sdata->ledvdd);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int stmfts_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	int err;
> +	struct stmfts_data *sdata;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
> +						I2C_FUNC_SMBUS_BYTE_DATA |
> +						I2C_FUNC_SMBUS_I2C_BLOCK))
> +		return -ENODEV;
> +
> +	if (!client->dev.of_node)
> +		return -ENOENT;
> +
> +	sdata = devm_kzalloc(&client->dev, sizeof(*sdata), GFP_KERNEL);
> +	if (!sdata)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, sdata);
> +
> +	mutex_init(&sdata->mutex);
> +
> +	sdata->regulators[STMFTS_REGULATOR_VDD].supply = "vdd";
> +	sdata->regulators[STMFTS_REGULATOR_AVDD].supply = "avdd";
> +	err = devm_regulator_bulk_get(&client->dev,
> +			ARRAY_SIZE(sdata->regulators), sdata->regulators);
> +	if (err)
> +		return err;
> +
> +	err = devm_add_action_or_reset(&client->dev, stmfts_power_off, sdata);
> +	if (err)
> +		return err;
> +
> +	sdata->client = client;
> +
> +	init_completion(&sdata->signal);
> +
> +	/*
> +	 * Do not enable interrupts by default.
> +	 * One possible case when an IRQ can be already rased is e.g. if the
> +	 * regulator is set as always on and the stmfts device sends an IRQ as
> +	 * soon as it gets powered, de-synchronizing the power on sequence.
> +	 * During power on, the device will be reset and all the initialization
> +	 * IRQ will be resent.
> +	 */
> +	irq_set_status_flags(client->irq, IRQ_NOAUTOEN);
> +	err = devm_request_threaded_irq(&client->dev, client->irq,
> +					NULL, stmfts_irq_handler,
> +					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> +					"stmfts_irq", sdata);
> +	if (err)
> +		return err;
> +
> +	dev_info(&client->dev, "initializing ST-Microelectronics FTS...\n");
> +	err = stmfts_power_on(sdata);
> +	if (err)
> +		return err;
> +
> +	sdata->use_key = of_property_read_bool(client->dev.of_node,
> +						"touch-key-connected");
> +
> +	sdata->input = devm_input_allocate_device(&client->dev);
> +	if (!sdata->input)
> +		return -ENOMEM;
> +
> +	sdata->input->name = STMFTS_DEV_NAME;
> +	sdata->input->id.bustype = BUS_I2C;
> +	sdata->input->open = stmfts_input_open;
> +	sdata->input->close = stmfts_input_close;
> +
> +	touchscreen_parse_properties(sdata->input, true, &sdata->prop);
> +
> +	input_set_abs_params(sdata->input, ABS_MT_POSITION_X, 0,
> +						sdata->prop.max_x, 0, 0);
> +	input_set_abs_params(sdata->input, ABS_MT_POSITION_Y, 0,
> +						sdata->prop.max_y, 0, 0);
> +	input_set_abs_params(sdata->input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input, ABS_MT_TOUCH_MINOR, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input, ABS_MT_ORIENTATION, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +	input_set_abs_params(sdata->input, ABS_DISTANCE, 0, 255, 0, 0);
> +
> +	if (sdata->use_key) {
> +		input_set_capability(sdata->input, EV_KEY, KEY_MENU);
> +		input_set_capability(sdata->input, EV_KEY, KEY_BACK);
> +	}
> +
> +	err = input_mt_init_slots(sdata->input,
> +				STMFTS_MAX_FINGERS, INPUT_MT_DIRECT);
> +	if (err)
> +		return err;
> +
> +	input_set_drvdata(sdata->input, sdata);
> +	err = input_register_device(sdata->input);
> +	if (err)
> +		return err;
> +
> +	if (sdata->use_key) {
> +		err = stmfts_enable_led(sdata);
> +		if (err) {
> +			/* even if the LEDs have failed to be initialized and
> +			 * used in the driver, I can still use the device even
> +			 * without LEDs. The ledvdd regulator pointer will be
> +			 * used as a flag.
> +			 */
> +			dev_warn(&client->dev,
> +					"unable to use touchkey leds\n");
> +			sdata->ledvdd = NULL;
> +		}
> +	}
> +
> +	err = sysfs_create_group(&sdata->client->dev.kobj,
> +					&stmfts_attribute_group);
> +	if (err)
> +		return err;
> +
> +	pm_runtime_enable(&client->dev);
> +
> +	return 0;
> +}
> +
> +static int stmfts_remove(struct i2c_client *client)
> +{
> +	pm_runtime_disable(&client->dev);
> +	sysfs_remove_group(&client->dev.kobj, &stmfts_attribute_group);
> +
> +	return 0;
> +}
> +
> +static int stmfts_runtime_suspend(struct device *dev)
> +{
> +	int ret;
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	ret = i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);
> +	if (ret)
> +		dev_warn(dev, "failed to suspend device\n");
> +
> +	return ret;
> +}
> +
> +static int stmfts_runtime_resume(struct device *dev)
> +{
> +	int ret;
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	ret = i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_OUT);
> +	if (ret)
> +		dev_err(dev, "failed to resume device\n");
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused stmfts_suspend(struct device *dev)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	stmfts_power_off(sdata);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused stmfts_resume(struct device *dev)
> +{
> +	struct stmfts_data *sdata = dev_get_drvdata(dev);
> +
> +	return stmfts_power_on(sdata);
> +}
> +
> +static const struct dev_pm_ops stmfts_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(stmfts_suspend, stmfts_resume)
> +	SET_RUNTIME_PM_OPS(stmfts_runtime_suspend, stmfts_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id stmfts_of_match[] = {
> +	{ .compatible = "st,stmfts", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, stmfts_of_match);
> +
> +static const struct i2c_device_id stmfts_id[] = {
> +	{ "stmfts", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, stmfts_id);
> +
> +static struct i2c_driver stmfts_driver = {
> +	.driver = {
> +		.name = STMFTS_DEV_NAME,
> +		.of_match_table = of_match_ptr(stmfts_of_match),
> +		.pm = &stmfts_pm_ops,
> +	},
> +	.probe = stmfts_probe,
> +	.remove = stmfts_remove,
> +	.id_table = stmfts_id,
> +};
> +
> +module_i2c_driver(stmfts_driver);
> +
> +MODULE_AUTHOR("Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics FTS Touch Screen");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.11.0
> 
--
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

^ permalink raw reply

* Re: [PATCH v7 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Benjamin Herrenschmidt @ 2017-04-25  2:21 UTC (permalink / raw)
  To: Brendan Higgins, wsa, robh+dt, mark.rutland, tglx, jason,
	marc.zyngier, joel, vz, mouse, clg
  Cc: linux-i2c, devicetree, linux-kernel, openbmc
In-Reply-To: <20170424181818.2754-5-brendanhiggins@google.com>

On Mon, 2017-04-24 at 11:18 -0700, Brendan Higgins wrote:
> +static int __aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> +                                struct platform_device *pdev)
> +{

Minor nit ... I'm really not fan of those underscores.

We use __ functions in some cases in the kernel for low level
helpers, usually when it's a low level variant of an existing
function or an "unlocked" variant, but I don't think generalizing
it to pretty much everything in the driver is worthwhile here.

If you want to be explicit about locking, I would suggest you
use a comment in front of the function explaining if it
expects to be called with the lock held.

We tend to only do that when *both* functions exist and one is
implemented in term of the other.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Benjamin Herrenschmidt @ 2017-04-25  2:19 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
	Kachalov Anton, Cédric Le Goater, linux-i2c, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist
In-Reply-To: <CAFd5g45DQZz99y59AY-4465qnM+tRv6+bDdGQqS6VYHEfRfjDg@mail.gmail.com>

On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
> > > +struct aspeed_i2c_bus {
> > > +     struct i2c_adapter              adap;
> > > +     struct device                   *dev;
> > > +     void __iomem                    *base;
> > > +     /* Synchronizes I/O mem access to base. */
> > > +     spinlock_t                      lock;
> > 
> > I am not entirely convinced we need that lock. The i2c core will
> > take a mutex protecting all operations on the bus. So we only need
> > to synchronize between our "xfer" code and our interrupt handler.
> 
> You are right if both having slave and master active at the same time
> was not possible; however, it is.

Right, I somewhat forgot about the slave case.

  ...

> > Some of those error states probably also warrant a reset of the
> > controller,
> > I think aspeed does that in the SDK.
> 
> For timeout and cmd_err, I do not see any argument against it; it
> sounds like we are in a very messed up, very unknown state, so full
> reset is probably the best last resort.

Yup.

> For SDA staying pulled down, I
> think we can say with reasonable confidence that some device on our
> bus is behaving very badly and I am not convinced that resetting the
> controller is likely to do anything to help;

Right. Hammering with STOPs and pray ...

>  that being said, I really
> do not have any good ideas to address that. So maybe praying and
> resetting the controller is *the most reasonable thing to do.* I
> would like to know what you think we should do in that case.

Well, there's a (small ?) chance that it's a controller bug asserting
the line so ... but there's little we can do if not.

> While I was thinking about this I also realized that the SDA line
> check after recovery happens in the else branch, but SCL line check
> does not happen after we attempt to STOP if SCL is hung. If we decide
> to make special note SDA being hung by a device that won't let go, we
> might want to make a special note that SCL is hung by a device that
> won't let go. Just a thought.

Maybe. Or just "unrecoverable error"... hopefully these don't happen
too often ... We had cases of a TPM misbehaving like that.

> > > +out:
> 
> ...
> > What about I2C_M_NOSTART ?
> > 
> > Not that I've ever seen it used... ;-)
> 
> Right now I am not doing any of the protocol mangling options, but I
> can add them in if you think it is important for initial support.

No, not important, we can add that later if it ever becomes useful.

 ...

> > In general, you always ACK all interrupts first. Then you handle
> > the bits you have harvested.
> > 
> 
> The documentation says to ACK the interrupt after handling in the RX
> case:
> 
> <<<
> S/W needs to clear this status bit to allow next data receiving.
> > > > 
> 
> I will double check with Ryan to make sure TX works the same way.
> 
> > > +     if (irq_status & ASPEED_I2CD_INTR_ERROR ||
> > > +         (!bus->msgs && bus->master_state !=
> > > ASPEED_I2C_MASTER_STOP)) {
> 
> ...
> > 
> > I would set master_state to "RECOVERY" (new state ?) and ensure
> > those things are caught if they happen outside of a recovery.

I replied privately ... as long as we ack before we start a new command
we should be ok but we shouldn't ack after.

Your latest patch still does that. It will do things like start a STOP
command *then* ack the status bits. I'm pretty sure that's bogus.

That way it's a lot simpler to simply move the

	writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);

To either right after the readl of the status reg at the beginning of
aspeed_i2c_master_irq().

I would be very surprised if that didn't work properly and wasn't much
safer than what you are currently doing. 

> Let me know if you still think we need a "RECOVERY" state.

The way you just switch to stop state and store the error for later
should work I think.

> > 
> > > +     if (bus->master_state == ASPEED_I2C_MASTER_START) {
> 
> ...
> > 
> > > +                     dev_dbg(bus->dev,
> > > +                             "no slave present at %02x", msg-
> > > >addr);
> > > +                     status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> > > +                     bus->cmd_err = -EIO;
> > > +                     do_stop(bus);
> > > +                     goto out_no_complete;
> > > +             } else {
> > > +                     status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> > > +                     if (msg->flags & I2C_M_RD)
> > > +                             bus->master_state =
> > > ASPEED_I2C_MASTER_RX;
> > > +                     else
> > > +                             bus->master_state =
> > > ASPEED_I2C_MASTER_TX_FIRST;
> > 
> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need
> > to handle this here ? A quick look at the TX_FIRST case makes
> > me think we are ok there but I'm not sure about the RX case.
> 
> I did not think that there is an SMBUS_QUICK RX. Could you point me
> to an example?

Not so much an RX, it's more like you are sending a 1-bit data in
the place of the Rd/Wr bit. So you have a read with a lenght of 0,
I don't think in that case you should set ASPEED_I2CD_M_RX_CMD in
__aspeed_i2c_do_start

> > I'm not sure the RX case is tight also. What completion does the
> > HW give you for the address cycle ? Won't you get that before it
> > has received the first character ? IE. You fall through to
> > the read case of the state machine with the read potentially
> > not complete yet no ?
> 
> ...
> > > +     case ASPEED_I2C_MASTER_RX:
> > > +             if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
> > > +                     dev_err(bus->dev, "master failed to RX");
> > > +                     goto out_complete;
> > > +             }
> > 
> > See my comment above for a bog standard i2c_read. Aren't you
> > getting
> > the completion for the address before the read is even started ?
> 
> In practice no, but it is probably best to be safe :-)

Yup :)
> > 
> > > +             status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> > > +
> > > +             recv_byte = aspeed_i2c_read(bus,
> > > ASPEED_I2C_BYTE_BUF_REG) >> 8;
> > > +             msg->buf[bus->buf_index++] = recv_byte;
> > > +
> > > +             if (msg->flags & I2C_M_RECV_LEN &&
> > > +                 recv_byte <= I2C_SMBUS_BLOCK_MAX) {
> > > +                     msg->len = recv_byte +
> > > +                                     ((msg->flags &
> > > I2C_CLIENT_PEC) ? 2 : 1);
> 
> ...
> > > +     return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
> > > +             & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
> > > +                     | ((clk_low <<
> > > ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
> > > +                        & ASPEED_I2CD_TIME_SCL_LOW_MASK)
> > > +                     | (base_clk &
> > > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
> > > +}
> > 
> > As I think I mentioned earlier, the AST2500 has a slightly
> > different
> > register layout which support larger values for high and low, thus
> > allowing a finer granularity.
> 
> I am developing against the 2500.

Yes but we'd like the driver to work with both :-)

> > BTW. In case you haven't, I would suggest you copy/paste the above
> > in
> > a userspace app and run it for all frequency divisors and see if
> > your
> > results match the aspeed table :)
> 
> Good call.

If you end up doing that, can you shoot it my way ? I can take care
of making sure it's all good for the 2400.

> > > +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> > > +                            struct platform_device *pdev)
> > > +{
> > > +     u32 clk_freq, divisor;
> > > +     struct clk *pclk;
> > > +     int ret;
> > > +
> > > +     pclk = devm_clk_get(&pdev->dev, NULL);
> > > +     if (IS_ERR(pclk)) {
> > > +             dev_err(&pdev->dev, "clk_get failed\n");
> > > +             return PTR_ERR(pclk);
> > > +     }
> > > +     ret = of_property_read_u32(pdev->dev.of_node,
> > > +                                "clock-frequency", &clk_freq);
> > 
> > See my previous comment about calling that 'bus-frequency' rather
> > than 'clock-frequency'.
> > 
> > > +     if (ret < 0) {
> > > +             dev_err(&pdev->dev,
> > > +                     "Could not read clock-frequency
> > > property\n");
> > > +             clk_freq = 100000;
> > > +     }
> > > +     divisor = clk_get_rate(pclk) / clk_freq;
> > > +     /* We just need the clock rate, we don't actually use the
> > > clk object. */
> > > +     devm_clk_put(&pdev->dev, pclk);
> > > +
> > > +     /* Set AC Timing */
> > > +     if (clk_freq / 1000 > 1000) {
> > > +             aspeed_i2c_write(bus, aspeed_i2c_read(bus,
> > > +                                                   ASPEED_I2C_FU
> > > N_CTRL_REG) |
> > > +                             ASPEED_I2CD_M_HIGH_SPEED_EN |
> > > +                             ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
> > > +                             ASPEED_I2CD_SDA_DRIVE_1T_EN,
> > > +                             ASPEED_I2C_FUN_CTRL_REG);
> > > +
> > > +             aspeed_i2c_write(bus, 0x3,
> > > ASPEED_I2C_AC_TIMING_REG2);
> > > +             aspeed_i2c_write(bus,
> > > aspeed_i2c_get_clk_reg_val(divisor),
> > > +                              ASPEED_I2C_AC_TIMING_REG1);
> > 
> > I already discussed by doubts about the above. I can try to scope
> > it with the EVB if you don't get to it. For now I'd rather take the
> > code out.
> > 
> > We should ask aspeed from what frequency the "1T" stuff is useful.
> 
> Will do, I will try to rope Ryan in on the next review; it will be
> good for him to get used to working with upstream anyway.

Yup. However, for the sake of getting something upstream (and in
OpenBMC 4.10 kernel) asap, I would suggest just dropping support
for those fast speeds for now, we can add them back later.

> > 
> > > +     } else {
> > > +             aspeed_i2c_write(bus,
> > > aspeed_i2c_get_clk_reg_val(divisor),
> > > +                              ASPEED_I2C_AC_TIMING_REG1);
> > > +             aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
> > > +                              ASPEED_I2C_AC_TIMING_REG2);
> > > +     }
> 
> ...
> > > +     spin_lock_init(&bus->lock);
> > > +     init_completion(&bus->cmd_complete);
> > > +     bus->adap.owner = THIS_MODULE;
> > > +     bus->adap.retries = 0;
> > > +     bus->adap.timeout = 5 * HZ;
> > > +     bus->adap.algo = &aspeed_i2c_algo;
> > > +     bus->adap.algo_data = bus;
> > > +     bus->adap.dev.parent = &pdev->dev;
> > > +     bus->adap.dev.of_node = pdev->dev.of_node;
> > > +     snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed
> > > i2c");
> > 
> > Another trivial one, should we put some kind of bus number
> > in that string ?
> 
> Whoops, looks like I missed this one; I will get to it in the next
> revision.

Ok. I noticed you missed that in v7, so I assume you mean v8 :-)

> > 
> > > +     bus->dev = &pdev->dev;
> > > +
> > > +     /* reset device: disable master & slave functions */
> > > +     aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
> 
> ...
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox