Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] dt-bindings: pinctrl: Add bindings for mscc,ocelot-sgpio
From: Linus Walleij @ 2020-05-27 13:45 UTC (permalink / raw)
  To: Lars Povlsen
  Cc: SoC Team, Rob Herring, Microchip Linux Driver Support,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:GPIO SUBSYSTEM, Linux ARM, linux-kernel@vger.kernel.org,
	Alexandre Belloni
In-Reply-To: <87pnappzun.fsf@soft-dev15.microsemi.net>

On Wed, May 27, 2020 at 10:05 AM Lars Povlsen
<lars.povlsen@microchip.com> wrote:

> The only issue is that the gpios on the same "port" have restrictions on
> their status - they can only be enabled "all" or "none" for gpios that
> map to the same port. F.ex. gpio0, gpio32, gpio64 and gpio96 must all be
> enabled or disabled because at the hardware level you control the
> _port_.

This is fairly common. For example that an entire port/block share
a clock.

> But as I noted earlier, that could just be the driver enforcing
> this.

Yeps.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v4 07/11] i2c: designware: Discard Cherry Trail model flag
From: Andy Shevchenko @ 2020-05-27 13:43 UTC (permalink / raw)
  To: Serge Semin
  Cc: Jarkko Nikula, Wolfram Sang, Mika Westerberg, Serge Semin,
	Alexey Malahov, Thomas Bogendoerfer, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527120111.5781-8-Sergey.Semin@baikalelectronics.ru>

On Wed, May 27, 2020 at 03:01:07PM +0300, Serge Semin wrote:
> A PM workaround activated by the flag MODEL_CHERRYTRAIL has been removed
> since commit 9cbeeca05049 ("i2c: designware: Remove Cherry Trail PMIC I2C
> bus pm_disabled workaround"), but the flag most likely by mistake has been
> left in the Dw I2C drivers. Let's remove it.

...

> -#define MODEL_MSCC_OCELOT	0x00000200
> +#define MODEL_MSCC_OCELOT	0x00000100

But why?

Does 0x200 work or not? I didn't see this in commit message.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v4 06/11] i2c: designware: Add Baytrail sem config DW I2C platform dependency
From: Andy Shevchenko @ 2020-05-27 13:42 UTC (permalink / raw)
  To: Serge Semin
  Cc: Jarkko Nikula, Wolfram Sang, Serge Semin, Alexey Malahov,
	Thomas Bogendoerfer, Mika Westerberg, Rob Herring, linux-mips,
	devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527120111.5781-7-Sergey.Semin@baikalelectronics.ru>

On Wed, May 27, 2020 at 03:01:06PM +0300, Serge Semin wrote:
> Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C
> platform driver. It's a bit confusing to see it's config in the menu at
> some separated place with no reference to the platform code. Let's move the
> config definition to be below the I2C_DESIGNWARE_PLATFORM config and mark
> it with "depends on I2C_DESIGNWARE_PLATFORM" statement. By doing so the
> config menu will display the feature right below the DW I2C platform
> driver item and will indent it to the right so signifying its belonging.

...

>  config I2C_DESIGNWARE_BAYTRAIL
>  	bool "Intel Baytrail I2C semaphore support"
>  	depends on ACPI
> +	depends on I2C_DESIGNWARE_PLATFORM
>  	depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
>  		   (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)

I didn't get this. What is broken now with existing dependencies?

(The move of the PCI part is fine)

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v25 01/16] dt: bindings: Add multicolor class dt bindings documention
From: Pavel Machek @ 2020-05-27 13:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: Dan Murphy, jacek.anaszewski, devicetree, linux-leds,
	linux-kernel
In-Reply-To: <20200527014902.GA859634@bogus>

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

Hi!

Thanks for reviews!

> > +additionalProperties: false
> > +...
> > diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> 
> This isn't a binding file. Belongs in another patch.

These constants are directly related to the binding. It makes sense to
go in one patch...

Best regards,
								Pavel

> > index f1f718dbe0f8..846248a0693d 100644
> > --- a/drivers/leds/led-core.c
> > +++ b/drivers/leds/led-core.c
> > @@ -34,6 +34,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
> >  	[LED_COLOR_ID_VIOLET] = "violet",
> >  	[LED_COLOR_ID_YELLOW] = "yellow",
> >  	[LED_COLOR_ID_IR] = "ir",
> > +	[LED_COLOR_ID_MULTI] = "multicolor",
> >  };
> >  EXPORT_SYMBOL_GPL(led_colors);
> >  
> > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> > index 0ce7dfc00dcb..a463ce6a8794 100644
> > --- a/include/dt-bindings/leds/common.h
> > +++ b/include/dt-bindings/leds/common.h
> > @@ -30,7 +30,8 @@
> >  #define LED_COLOR_ID_VIOLET	5
> >  #define LED_COLOR_ID_YELLOW	6
> >  #define LED_COLOR_ID_IR		7
> > -#define LED_COLOR_ID_MAX	8
> > +#define LED_COLOR_ID_MULTI	8
> > +#define LED_COLOR_ID_MAX	9
> >  
> >  /* Standard LED functions */
> >  /* Keyboard LEDs, usually it would be input4::capslock etc. */
> > -- 
> > 2.25.1
> > 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH] check: Add 10bit/slave i2c reg flags support
From: Andy Shevchenko @ 2020-05-27 13:36 UTC (permalink / raw)
  To: Serge Semin
  Cc: devicetree-compiler, Serge Semin, Alexey Malahov,
	Thomas Bogendoerfer, Wolfram Sang, Jarkko Nikula, Frank Rowand,
	Rob Herring, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527122525.6929-1-Sergey.Semin@baikalelectronics.ru>

On Wed, May 27, 2020 at 03:25:25PM +0300, Serge Semin wrote:
> Recently the I2C-controllers slave interface support was added to the
> kernel I2C subsystem. In this case Linux can be used as, for example,
> a I2C EEPROM machine. See [1] for details. Other than instantiating
> the EEPROM-slave device from user-space there is a way to declare the
> device in dts. In this case firstly the I2C bus controller must support
> the slave interface. Secondly I2C-slave sub-node of that controller
> must have "reg"-property with flag I2C_OWN_SLAVE_ADDRESS set (flag is
> declared in [2]). That flag is declared as (1 << 30), which when set
> makes dtc unhappy about too big address set for a I2C-slave:
> 
> Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
> 
> Similar problem would have happened if we had set the 10-bit address
> flag I2C_TEN_BIT_ADDRESS in the "reg"-property.
> 
> In order to fix the problem we suggest to alter the I2C-bus reg-check
> algorithm, so one would be aware of the upper bits set. Normally if no
> flag specified, the 7-bit address is expected in the "reg"-property.
> If I2C_TEN_BIT_ADDRESS is set, then the 10-bit address check will be
> performed. The I2C_OWN_SLAVE_ADDRESS flag will be just ignored.
> 
> [1] kernel/Documentation/i2c/slave-interface.rst
> [2] kernel/include/dt-bindings/i2c/i2c.h

...

> +		addr = reg & 0x3FFFFFFFU;
> +		if ((reg & (1 << 31)) && addr > 0x3ff)
>  			FAIL_PROP(c, dti, node, prop, "I2C address must be less than 10-bits, got \"0x%x\"",
> -				  reg);
> -
> +				  addr);
> +		else if (!(reg & (1 << 31)) && addr > 0x7f)
> +			FAIL_PROP(c, dti, node, prop, "I2C address must be less than 7-bits, got \"0x%x\"",
> +				  addr);

1 << 31 is UB.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v3 03/12] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Andy Shevchenko @ 2020-05-27 13:32 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Rob Herring,
	Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, Linux Kernel Mailing List
In-Reply-To: <20200527120716.4dke6psqpw726wrd@mobilestation>

On Wed, May 27, 2020 at 03:07:16PM +0300, Serge Semin wrote:
> On Wed, May 27, 2020 at 12:30:04PM +0300, Andy Shevchenko wrote:
> > On Wed, May 27, 2020 at 1:00 AM Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > >
> > > dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
> > > i2c "reg" property. If it is the compiler will print a warning:
> > 
> > Shouldn't be dtc whatever tools fixed?
> 
> See the first patch in the series.

I can't by the reason that I have no such. I also answered to cover letter
about it.

> > > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> > > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
> > >
> > > In order to silence dtc up let's discard the flag from the DW I2C DT
> > > binding example for now. Just revert this commit when dtc is fixed.
> > 
> > Doesn't sound like a good idea. If user happens in between of these
> > ping-pong change, how they will know this subtle issue?
> 
> As I see it, there are three ways we can follow.
> 1) Apply the patch and revert when dtc is fixed.
> 2) Apply the patch, but add a comment above the property, that we need
>    to get the 0x40000064 address back when dtc is dixed.
> 3) Leave this ugly warning be until dtc is fixed.
> 
> In a comment to v2 Rob mentioned a solution like 1). Personally I am ok with
> either, though I'd like to see a Rob's final comment about this.

Yes, let's follow what Rob proposes.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v3 00/12] i2c: designeware: Add Baikal-T1 System I2C support
From: Andy Shevchenko @ 2020-05-27 13:30 UTC (permalink / raw)
  To: Serge Semin
  Cc: Jarkko Nikula, Wolfram Sang, Serge Semin, Alexey Malahov,
	Maxim Kaurkin, Pavel Parkhomenko, Ramil Zaripov,
	Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
	Thomas Bogendoerfer, Mika Westerberg, Rob Herring, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200526215528.16417-1-Sergey.Semin@baikalelectronics.ru>

On Wed, May 27, 2020 at 12:55:16AM +0300, Serge Semin wrote:
> Jarkko, Wolfram, the merge window is upon us, please review/merge in/whatever
> the patchset.
> 
> Initially this has been a small patchset which embedded the Baikal-T1
> System I2C support into the DW APB I2C driver as is by using a simplest
> way. After a short discussion with Andy we decided to implement what he
> suggested (introduce regmap-based accessors and create a glue driver) and
> even more than that to provide some cleanups of the code. So here is what
> this patchset consists of.
> 
> First of all we've found out that current implementation of scripts/dtc
> didn't support i2c dt nodes with 10bit and slave flags set in the
> reg property. You'll see an error if you try to dt_binding_check it.
> So the very first patch fixes the problem by adding these flags support
> into the check_i2c_bus_reg() method.
> 
> Traditionally we converted the plain text-based DT binding to the DT schema
> and added Baikal-T1 System I2C device support there. This required to mark
> the reg property redundant for Baikal-T1 I2C since its reg-space is
> indirectly accessed by means of the System Controller cmd/read/write
> registers.
> 
> Then as Andy suggested we replaced the Synopsys DW APB I2C common driver
> registers IO accessors into the regmap API methods. This doesn't change
> the code logic much, though in two places we managed to replace some bulky
> peaces of code with a ready-to-use regmap methods.
> 
> Additionally before adding the glue layer API we initiated a set of cleanups:
> - Define components of the multi-object drivers (like i2c-designware-core.o
>   and i2c-designware-paltform.o) with using `-y` suffixed makefile
>   variables instead of `-objs` suffixed one. This is encouraged by
>   Documentation/kbuild/makefiles.rst text since `-objs` is supposed to be used
>   to build host programs.
> - Make DW I2C slave driver depended on the DW I2C core code instead of the
>   platform one, which it really is.
> - Move Intel Baytrail semaphore feature to the platform if-clause of the
>   kernel config.
> 
> After this we finally can introduce the glue layer API for the DW APB I2C
> platform driver. So there are three methods exported from the driver:
> i2c_dw_plat_setup(), i2c_dw_plat_clear(), &i2c_dw_plat_dev_pm_ops to
> setup, cleanup and add PM operations to the glue driven I2C device. Before
> setting the platform DW I2C device up the glue probe code is supposed to
> create an instance of DW I2C device generic object and pre-initialize
> its `struct device` pointer together with optional platform-specific
> flags. In addition to that we converted the MSCC Ocelot SoC I2C specific
> code into the glue layer seeing it's really too specific and, which is more
> important, isn't that complicated so we could unpin it without much of
> worrying to break something.
> 
> Meanwhile we discovered that MODEL_CHERRYTRAIL and MODEL_MASK actually
> were no longer used in the code. MODEL_MSCC flag has been discarded since
> the MSCC Ocelot I2C code conversion to the glue driver. So now we can get
> rid of all the MODEL-specific flags.
> 
> Finally we introduced a glue driver with Baikal-T1 System I2C device
> support. The driver probe tries to find a syscon regmap, creates the DW
> APB I2C regmap based on it and passes it further to the DW I2C device
> descriptor. Then it does normal DW APB I2C platform setup by calling a
> generic setup method. Cleanup is straightforward. It's just calling a
> generic DW APB I2C clean method.
> 
> This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
> base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
> tag: v5.7-rc4
> 
> Note new vendor prefix for Baikal-T1 System I2C device will be added in
> the framework of the next patchset:
> https://lkml.org/lkml/2020/5/6/1047

>   scripts/dtc: check: Add 10bit/slave i2c reg flags support

Hmm... I don't see this patch.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* [PATCH] ARM: dts: r8a7742-iwg21d-q7-dbcm-ca: Add device tree for camera DB
From: Lad Prabhakar @ 2020-05-27 13:29 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Magnus Damm
  Cc: devicetree, linux-kernel, linux-renesas-soc, Prabhakar,
	Lad Prabhakar

Add support for the camera daughter board which is connected to
iWave's RZ/G1H Qseven carrier board. Also enable ttySC[0135] and
ethernet1 interfaces.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 arch/arm/boot/dts/Makefile                      |  1 +
 arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts | 94 +++++++++++++++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index ed3376c..118e35c 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -918,6 +918,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \
 	r8a73a4-ape6evm.dtb \
 	r8a7740-armadillo800eva.dtb \
 	r8a7742-iwg21d-q7.dtb \
+	r8a7742-iwg21d-q7-dbcm-ca.dtb \
 	r8a7743-iwg20d-q7.dtb \
 	r8a7743-iwg20d-q7-dbcm-ca.dtb \
 	r8a7743-sk-rzg1m.dtb \
diff --git a/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
new file mode 100644
index 0000000..857aab5
--- /dev/null
+++ b/arch/arm/boot/dts/r8a7742-iwg21d-q7-dbcm-ca.dts
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the iWave-RZ/G1H Qseven board development
+ * platform with camera daughter board
+ *
+ * Copyright (C) 2020 Renesas Electronics Corp.
+ */
+
+/dts-v1/;
+#include "r8a7742-iwg21d-q7.dts"
+
+/ {
+	model = "iWave Systems RZ/G1H Qseven development platform with camera add-on";
+	compatible = "iwave,g21d", "iwave,g21m", "renesas,r8a7742";
+
+	aliases {
+		serial0 = &scif0;
+		serial1 = &scif1;
+		serial3 = &scifb1;
+		serial5 = &hscif0;
+		ethernet1 = &ether;
+	};
+};
+
+&avb {
+	/* Pins shared with VIN0, keep status disabled */
+	status = "disabled";
+};
+
+&ether {
+	pinctrl-0 = <&ether_pins>;
+	pinctrl-names = "default";
+
+	phy-handle = <&phy1>;
+	renesas,ether-link-active-low;
+	status = "okay";
+
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+		micrel,led-mode = <1>;
+	};
+};
+
+&hscif0 {
+	pinctrl-0 = <&hscif0_pins>;
+	pinctrl-names = "default";
+	uart-has-rtscts;
+	status = "okay";
+};
+
+&pfc {
+	ether_pins: ether {
+		groups = "eth_mdio", "eth_rmii";
+		function = "eth";
+	};
+
+	hscif0_pins: hscif0 {
+		groups = "hscif0_data", "hscif0_ctrl";
+		function = "hscif0";
+	};
+
+	scif0_pins: scif0 {
+		groups = "scif0_data";
+		function = "scif0";
+	};
+
+	scif1_pins: scif1 {
+		groups = "scif1_data";
+		function = "scif1";
+	};
+
+	scifb1_pins: scifb1 {
+		groups = "scifb1_data";
+		function = "scifb1";
+	};
+};
+
+&scif0 {
+	pinctrl-0 = <&scif0_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&scif1 {
+	pinctrl-0 = <&scif1_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&scifb1 {
+	pinctrl-0 = <&scifb1_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+};
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v13 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem
From: Laurent Pinchart @ 2020-05-27 13:23 UTC (permalink / raw)
  To: Vishal Sagar
  Cc: Hyun Kwon, mchehab@kernel.org, robh+dt@kernel.org,
	mark.rutland@arm.com, Michal Simek, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, hans.verkuil@cisco.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Dinesh Kumar, Sandip Kothari,
	Luca Ceresoli, Jacopo Mondi, Rob Herring
In-Reply-To: <DM6PR02MB68762DFB8F24E485B9B302B6A7B10@DM6PR02MB6876.namprd02.prod.outlook.com>

Hi Vishal,

On Wed, May 27, 2020 at 11:53:01AM +0000, Vishal Sagar wrote:
> On Sunday, May 24, 2020 7:32 AM, Laurent Pinchart wrote:
> > On Tue, May 12, 2020 at 08:49:46PM +0530, Vishal Sagar wrote:
> > > Add bindings documentation for Xilinx MIPI CSI-2 Rx Subsystem.
> > >
> > > The Xilinx MIPI CSI-2 Rx Subsystem consists of a CSI-2 Rx controller,
> > > a D-PHY in Rx mode and a Video Format Bridge.
> > >
> > > Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> > > Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > v13
> > > - Based on Laurent's suggestions
> > > - Fixed the datatypes values as minimum and maximum
> > > - condition added for en-vcx property
> > >
> > > v12
> > > - Moved to yaml format
> > > - Update CSI-2 and D-PHY
> > > - Mention that bindings for D-PHY not here
> > > - reset -> video-reset
> > >
> > > v11
> > > - Modify compatible string from 4.0 to 5.0
> > >
> > > v10
> > > - No changes
> > >
> > > v9
> > > - Fix xlnx,vfb description.
> > > - s/Optional/Required endpoint property.
> > > - Move data-lanes description from Ports to endpoint property section.
> > >
> > > v8
> > > - Added reset-gpios optional property to assert video_aresetn
> > >
> > > v7
> > > - Removed the control name from dt bindings
> > > - Updated the example dt node name to csi2rx
> > >
> > > v6
> > > - Added "control" after V4L2_CID_XILINX_MIPICSISS_ACT_LANES as
> > > suggested by Luca
> > > - Added reviewed by Rob Herring
> > >
> > > v5
> > > - Incorporated comments by Luca Cersoli
> > > - Removed DPHY clock from description and example
> > > - Removed bayer pattern from device tree MIPI CSI IP
> > >   doesn't deal with bayer pattern.
> > >
> > > v4
> > > - Added reviewed by Hyun Kwon
> > >
> > > v3
> > > - removed interrupt parent as suggested by Rob
> > > - removed dphy clock
> > > - moved vfb to optional properties
> > > - Added required and optional port properties section
> > > - Added endpoint property section
> > >
> > > v2
> > > - updated the compatible string to latest version supported
> > > - removed DPHY related parameters
> > > - added CSI v2.0 related property (including VCX for supporting upto 16
> > >   virtual channels).
> > > - modified csi-pxl-format from string to unsigned int type where the value
> > >   is as per the CSI specification
> > > - Defined port 0 and port 1 as sink and source ports.
> > > - Removed max-lanes property as suggested by Rob and Sakari
> > > .../bindings/media/xilinx/xlnx,csi2rxss.yaml  | 226 ++++++++++++++++++
> > >  1 file changed, 226 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > > b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> > > new file mode 100644
> > > index 000000000000..b0885f461785
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yam
> > > +++ l
> > > @@ -0,0 +1,226 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/xilinx/xlnx,csi2rxss.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Xilinx MIPI CSI-2 Receiver Subsystem
> > > +
> > > +maintainers:
> > > +  - Vishal Sagar <vishal.sagar@xilinx.com>
> > > +
> > > +description: |
> > > +  The Xilinx MIPI CSI-2 Receiver Subsystem is used to capture MIPI
> > > +CSI-2
> > > +  traffic from compliant camera sensors and send the output as AXI4
> > > +Stream
> > > +  video data for image processing.
> > > +  The subsystem consists of a MIPI D-PHY in slave mode which captures
> > > +the
> > > +  data packets. This is passed along the MIPI CSI-2 Rx IP which
> > > +extracts the
> > > +  packet data. The optional Video Format Bridge (VFB) converts this
> > > +data to
> > > +  AXI4 Stream video data.
> > > +  For more details, please refer to PG232 Xilinx MIPI CSI-2 Receiver
> > Subsystem.
> > > +  Please note that this bindings includes only the MIPI CSI-2 Rx
> > > +controller
> > > +  and Video Format Bridge and not D-PHY.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +        - xlnx,mipi-csi2-rx-subsystem-5.0
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    description: List of clock specifiers
> > > +    items:
> > > +      - description: AXI Lite clock
> > > +      - description: Video clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: lite_aclk
> > > +      - const: video_aclk
> > > +
> > > +  xlnx,csi-pxl-format:
> > > +    description: |
> > > +      This denotes the CSI Data type selected in hw design.
> > > +      Packets other than this data type (except for RAW8 and
> > > +      User defined data types) will be filtered out.
> > > +      Possible values are as below -
> > > +      0x1e - YUV4228B
> > > +      0x1f - YUV42210B
> > > +      0x20 - RGB444
> > > +      0x21 - RGB555
> > > +      0x22 - RGB565
> > > +      0x23 - RGB666
> > > +      0x24 - RGB888
> > > +      0x28 - RAW6
> > > +      0x29 - RAW7
> > > +      0x2a - RAW8
> > > +      0x2b - RAW10
> > > +      0x2c - RAW12
> > > +      0x2d - RAW14
> > > +      0x2e - RAW16
> > > +      0x2f - RAW20
> > > +    allOf:
> > > +      - $ref: /schemas/types.yaml#/definitions/uint32
> > > +      - anyOf:
> > > +        - minimum: 0x1e
> > > +        - maximum: 0x24
> > > +        - minimum: 0x28
> > > +        - maximum: 0x2f
> > > +
> > > +  xlnx,vfb:
> > > +    type: boolean
> > > +    description: Present when Video Format Bridge is enabled in IP
> > > + configuration
> > > +
> > > +  xlnx,en-csi-v2-0:
> > > +    type: boolean
> > > +    description: Present if CSI v2 is enabled in IP configuration.
> > > +
> > > +  xlnx,en-vcx:
> > > +    type: boolean
> > > +    description: |
> > > +      When present, there are maximum 16 virtual channels, else only 4.
> > > +
> > > +  xlnx,en-active-lanes:
> > > +    type: boolean
> > > +    description: |
> > > +      Present if the number of active lanes can be re-configured at
> > > +      runtime in the Protocol Configuration Register. Otherwise all lanes,
> > > +      as set in IP configuration, are always active.
> > > +
> > > +  video-reset-gpios:
> > > +    description: Optional specifier for a GPIO that asserts video_aresetn.
> > > +    maxItems: 1
> > > +
> > > +  ports:
> > > +    type: object
> > > +
> > > +    properties:
> > > +      port@0:
> > > +        type: object
> > > +        description: |
> > > +          Input / sink port node, single endpoint describing the
> > > +          CSI-2 transmitter.
> > > +
> > > +        properties:
> > > +          reg:
> > > +            const: 0
> > > +
> > > +          endpoint:
> > > +            type: object
> > > +
> > > +            properties:
> > > +
> > > +              data-lanes:
> > > +                description: |
> > > +                  This is required only in the sink port 0 endpoint which
> > > +                  connects to MIPI CSI-2 source like sensor.
> > > +                  The possible values are -
> > > +                  1       - For 1 lane enabled in IP.
> > > +                  1 2     - For 2 lanes enabled in IP.
> > > +                  1 2 3   - For 3 lanes enabled in IP.
> > > +                  1 2 3 4 - For 4 lanes enabled in IP.
> > > +                items:
> > > +                  - const: 1
> > > +                  - const: 2
> > > +                  - const: 3
> > > +                  - const: 4
> > > +
> > > +              remote-endpoint: true
> > > +
> > > +            required:
> > > +              - data-lanes
> > > +              - remote-endpoint
> > > +
> > > +            additionalProperties: false
> > > +
> > > +        additionalProperties: false
> > > +
> > > +      port@1:
> > > +        type: object
> > > +        description: |
> > > +          Output / source port node, endpoint describing modules
> > > +          connected the CSI-2 receiver.
> > > +
> > > +        properties:
> > > +
> > > +          reg:
> > > +            const: 1
> > > +
> > > +          endpoint:
> > > +            type: object
> > > +
> > > +            properties:
> > > +
> > > +              remote-endpoint: true
> > > +
> > > +            required:
> > > +              - remote-endpoint
> > > +
> > > +            additionalProperties: false
> > > +
> > > +        additionalProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - clock-names
> > > +  - xlnx,csi-pxl-format
> > > +  - ports
> > > +
> > > +if:
> > > +  not:
> > > +    required:
> > > +      - xlnx,en-csi-v2-0
> > > +then:
> > > +  properties:
> > > +    xlnx,en-vcx: false
> > 
> > As I've just commented on v12, I think we should condition the xlnx,csi-pxl-
> > format property to xlnx,vfb being set. xlnx,csi-pxl-format should be removed
> > from the required properties above, and the following conditions added:
> > 
> > allOf:
> >   - if:
> >       required:
> >         - xlnx,vfb
> >     then:
> >       required:
> >         - xlnx,csi-pxl-format
> >     else:
> >       properties:
> >         xlnx,csi-pxl-format: false
> > 
> >   - if:
> >       not:
> >         required:
> >           - xlnx,en-csi-v2-0
> >     then:
> >       properties:
> >         xlnx,en-vcx: false
> > 
> > The 'allOf' is needed as you can't have two 'if' constructs at the top level.
> > 
> Thanks for sharing the explanation for this.
> Can you please share where I can get this info?

The json-schema specification is available at
https://json-schema.org/specification.html. allOf is defined in
https://json-schema.org/draft/2019-09/json-schema-core.html#allOf.

JSON schemas are expressed in JSON format, and YAML is a (more readable)
superset syntax of JSON. A YAML document contains lists and objects:

- this
- is
- a
- list

object:
  can: have
  properties:
    that: can
    be: other
    objects

An object is similar to a Python dictionary, it can't have multiple
entries with the same key. So having

if:
  required:
    - xlnx,vfb
then:
  required:
    - xlnx,csi-pxl-format
else:
  properties:
    xlnx,csi-pxl-format: false

if:
  not:
    required:
      - xlnx,en-csi-v2-0
then:
  properties:
    xlnx,en-vcx: false

at the top level is not valid, the same way that

properties:
  reg:
    maxItems: 1
  reg:
    maxItems: 1

wouldn't be valid. The allOf object has a value that is a list of
schemas:

allOf:
  - schema1
  - schema2
  - schema3

and in this case, we use it with a if...then...else for each of the
schemas. As documented in the spec, "An instance validates successfully
against [allOf] if it validates successfully against all schemas defined
by [allOf]'s value".

allOf is also used to include sub-schemas, as explained in
Documentation/devicetree/bindings/example-schema.yaml.

  vendor,int-property:
    description: Vendor specific properties must have a description
    # 'allOf' is the json-schema way of subclassing a schema. Here the base
    # type schema is referenced and then additional constraints on the values
    # are added.
    allOf:
      - $ref: /schemas/types.yaml#/definitions/uint32
      - enum: [2, 4, 6, 8, 10]

If this was written

  vendor,int-property:
    $ref: /schemas/types.yaml#/definitions/uint32
    enum: [2, 4, 6, 8, 10]

we would have an issue (among other problems) if
/schemas/types.yaml#/definitions/uint32 contained an enum, as there
would be two enum properties for vendor,int-property.

> > Please however let me know if my understanding is wrong and xlnx,csi-pxl-
> > format is needed even when xlnx,vfb is not set. In that case please ignore this
> > change (but please add the ... below).
> 
> Ok. I will add ... in the end.
> 
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/gpio/gpio.h>
> > > +    xcsi2rxss_1: csi2rx@a0020000 {
> > > +        compatible = "xlnx,mipi-csi2-rx-subsystem-5.0";
> > > +        reg = <0x0 0xa0020000 0x0 0x10000>;
> > > +        interrupt-parent = <&gic>;
> > > +        interrupts = <0 95 4>;
> > > +        xlnx,csi-pxl-format = <0x2a>;
> > > +        xlnx,vfb;
> > > +        xlnx,en-active-lanes;
> > > +        xlnx,en-csi-v2-0;
> > > +        xlnx,en-vcx;
> > > +        clock-names = "lite_aclk", "video_aclk";
> > > +        clocks = <&misc_clk_0>, <&misc_clk_1>;
> > > +        video-reset-gpios = <&gpio 86 GPIO_ACTIVE_LOW>;
> > > +
> > > +        ports {
> > > +            #address-cells = <1>;
> > > +            #size-cells = <0>;
> > > +
> > > +            port@0 {
> > > +                /* Sink port */
> > > +                reg = <0>;
> > > +                csiss_in: endpoint {
> > > +                    data-lanes = <1 2 3 4>;
> > > +                    /* MIPI CSI-2 Camera handle */
> > > +                    remote-endpoint = <&camera_out>;
> > > +                };
> > > +            };
> > > +            port@1 {
> > > +                /* Source port */
> > > +                reg = <1>;
> > > +                csiss_out: endpoint {
> > > +                    remote-endpoint = <&vproc_in>;
> > > +                };
> > > +            };
> > > +        };
> > > +    };
> > 
> > YAML files usually end with
> > 
> > ...
> > 
> > on the last line to mark the end of file.
> > 
> 
> Ok I will add this to the end of the file.
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Andrew Lunn @ 2020-05-27 13:12 UTC (permalink / raw)
  To: Dan Murphy
  Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
	devicetree
In-Reply-To: <c0867d48-6f04-104b-8192-d61d4464a65f@ti.com>

> If the dt defines rgmii-rx/tx-id then these values are required not
> optional.  That was the discussion on the binding.

How many times do i need to say it. They are optional. If not
specified, default to 2ns.

> > > +	ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
> > > +				   &dp83869->tx_id_delay);
> > > +	if (ret) {
> > > +		dp83869->tx_id_delay = ret;
> > > +		ret = 0;
> > > +	}
> > > +
> > >   	return ret;
> > >   }
> > >   #else
> > > @@ -367,10 +388,45 @@ static int dp83869_configure_mode(struct phy_device *phydev,
> > >   	return ret;
> > >   }
> > > +static int dp83869_get_delay(struct phy_device *phydev)
> > > +{
> > > +	struct dp83869_private *dp83869 = phydev->priv;
> > > +	int delay_size = ARRAY_SIZE(dp83869_internal_delay);
> > > +	int tx_delay = 0;
> > > +	int rx_delay = 0;
> > > +
> > > +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> > > +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> > > +		tx_delay = phy_get_delay_index(phydev,
> > > +					       &dp83869_internal_delay[0],
> > > +					       delay_size, dp83869->tx_id_delay,
> > > +					       false);
> > > +		if (tx_delay < 0) {
> > > +			phydev_err(phydev, "Tx internal delay is invalid\n");
> > > +			return tx_delay;
> > > +		}
> > > +	}
> > > +
> > > +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> > > +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> > > +		rx_delay = phy_get_delay_index(phydev,
> > > +					       &dp83869_internal_delay[0],
> > > +					       delay_size, dp83869->rx_id_delay,
> > > +					       false);
> > > +		if (rx_delay < 0) {
> > > +			phydev_err(phydev, "Rx internal delay is invalid\n");
> > > +			return rx_delay;
> > > +		}
> > > +	}
> > So any PHY using these properties is going to pretty much reproduce
> > this code. Meaning is should all be in a helper.
> 
> The issue here is that the phy_mode may only be rgmii-txid so you only want
> to find the tx_delay and return.
> 
> Same with the RXID.  How is the helper supposed to know what delay to return
> and look for?

How does this code do it? It looks at the value of interface.

    Andrew

^ permalink raw reply

* [PATCH 2/4] dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block
From: Sivaprakash Murugesan @ 2020-05-27 12:38 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, jassisinghbrar, linux-arm-msm,
	devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590583092-24290-1-git-send-email-sivaprak@codeaurora.org>

Add dt-bindings for ipq6018 mailbox driver

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 .../bindings/mailbox/qcom,apcs-kpss-global.yaml         | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
index 12eff94..e05bff4 100644
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -16,6 +16,7 @@ maintainers:
 properties:
   compatible:
     enum:
+      - qcom,ipq6018-apcs-apps-global
       - qcom,ipq8074-apcs-apps-global
       - qcom,msm8916-apcs-kpss-global
       - qcom,msm8996-apcs-hmss-global
@@ -38,12 +39,12 @@ properties:
     const: 1
 
   '#clock-cells':
-    const: 0
+    enum: [ 0, 1 ]
 
   clock-names:
     items:
       - const: pll
-      - const: aux
+      - enum: [ aux, xo ]
 
 required:
   - compatible
@@ -84,3 +85,15 @@ examples:
         clock-names = "pll", "aux";
         #clock-cells = <0>;
     };
+
+  # Example apcs with ipq6018
+  - |
+    #include "dt-bindings/clock/qcom,apss-ipq.h"
+    apcs_ipq: mailbox@b111000 {
+        compatible = "qcom,ipq6018-apcs-apps-global";
+        reg = <0x0b111000 0x1000>;
+        #clock-cells = <1>;
+        clocks = <&a53pll>, <&xo>;
+        clock-names = "pll", "xo";
+        #mbox-cells = <1>;
+    };
-- 
2.7.4


^ permalink raw reply related

* [PATCH 1/4] dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
From: Sivaprakash Murugesan @ 2020-05-27 12:38 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, jassisinghbrar, linux-arm-msm,
	devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590583092-24290-1-git-send-email-sivaprak@codeaurora.org>

Qualcomm APCS global block provides a bunch of generic properties which
are required in a device tree. Add YAML schema for these properties.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 .../bindings/mailbox/qcom,apcs-kpss-global.txt     | 88 ----------------------
 .../bindings/mailbox/qcom,apcs-kpss-global.yaml    | 86 +++++++++++++++++++++
 2 files changed, 86 insertions(+), 88 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
 create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
deleted file mode 100644
index beec612..0000000
--- a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
+++ /dev/null
@@ -1,88 +0,0 @@
-Binding for the Qualcomm APCS global block
-==========================================
-
-This binding describes the APCS "global" block found in various Qualcomm
-platforms.
-
-- compatible:
-	Usage: required
-	Value type: <string>
-	Definition: must be one of:
-		    "qcom,msm8916-apcs-kpss-global",
-		    "qcom,msm8996-apcs-hmss-global"
-		    "qcom,msm8998-apcs-hmss-global"
-		    "qcom,qcs404-apcs-apps-global"
-		    "qcom,sc7180-apss-shared"
-		    "qcom,sdm845-apss-shared"
-		    "qcom,sm8150-apss-shared"
-		    "qcom,ipq8074-apcs-apps-global"
-
-- reg:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: must specify the base address and size of the global block
-
-- clocks:
-	Usage: required if #clock-names property is present
-	Value type: <phandle array>
-	Definition: phandles to the two parent clocks of the clock driver.
-
-- #mbox-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: as described in mailbox.txt, must be 1
-
-- #clock-cells:
-	Usage: optional
-	Value type: <u32>
-	Definition: as described in clock.txt, must be 0
-
-- clock-names:
-	Usage: required if the platform data based clock driver needs to
-	retrieve the parent clock names from device tree.
-	This will requires two mandatory clocks to be defined.
-	Value type: <string-array>
-	Definition: must be "pll" and "aux"
-
-= EXAMPLE
-The following example describes the APCS HMSS found in MSM8996 and part of the
-GLINK RPM referencing the "rpm_hlos" doorbell therein.
-
-	apcs_glb: mailbox@9820000 {
-		compatible = "qcom,msm8996-apcs-hmss-global";
-		reg = <0x9820000 0x1000>;
-
-		#mbox-cells = <1>;
-	};
-
-	rpm-glink {
-		compatible = "qcom,glink-rpm";
-
-		interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
-
-		qcom,rpm-msg-ram = <&rpm_msg_ram>;
-
-		mboxes = <&apcs_glb 0>;
-		mbox-names = "rpm_hlos";
-	};
-
-Below is another example of the APCS binding on MSM8916 platforms:
-
-	apcs: mailbox@b011000 {
-		compatible = "qcom,msm8916-apcs-kpss-global";
-		reg = <0xb011000 0x1000>;
-		#mbox-cells = <1>;
-		clocks = <&a53pll>;
-		#clock-cells = <0>;
-	};
-
-Below is another example of the APCS binding on QCS404 platforms:
-
-	apcs_glb: mailbox@b011000 {
-		compatible = "qcom,qcs404-apcs-apps-global", "syscon";
-		reg = <0x0b011000 0x1000>;
-		#mbox-cells = <1>;
-		clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
-		clock-names = "pll", "aux";
-		#clock-cells = <0>;
-	};
diff --git a/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
new file mode 100644
index 0000000..12eff94
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mailbox/qcom,apcs-kpss-global.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm APCS global block bindings
+
+description:
+  This binding describes the APCS "global" block found in various Qualcomm
+  platforms.
+
+maintainers:
+  - Sivaprakash Murugesan <sivaprak@codeaurora.org>
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq8074-apcs-apps-global
+      - qcom,msm8916-apcs-kpss-global
+      - qcom,msm8996-apcs-hmss-global
+      - qcom,msm8998-apcs-hmss-global
+      - qcom,qcs404-apcs-apps-global
+      - qcom,sc7180-apss-shared
+      - qcom,sdm845-apss-shared
+      - qcom,sm8150-apss-shared
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: phandles to the parent clocks of the clock driver
+    items:
+      - description: primary pll parent of the clock driver
+      - description: auxiliary parent
+
+  '#mbox-cells':
+    const: 1
+
+  '#clock-cells':
+    const: 0
+
+  clock-names:
+    items:
+      - const: pll
+      - const: aux
+
+required:
+  - compatible
+  - reg
+  - '#mbox-cells'
+
+additionalProperties: false
+
+examples:
+
+  # Example apcs with msm8996
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    apcs_glb: mailbox@9820000 {
+        compatible = "qcom,msm8996-apcs-hmss-global";
+        reg = <0x9820000 0x1000>;
+
+        #mbox-cells = <1>;
+    };
+
+    rpm-glink {
+        compatible = "qcom,glink-rpm";
+        interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
+        qcom,rpm-msg-ram = <&rpm_msg_ram>;
+        mboxes = <&apcs_glb 0>;
+        mbox-names = "rpm_hlos";
+    };
+
+  # Example apcs with qcs404
+  - |
+    #define GCC_APSS_AHB_CLK_SRC  1
+    #define GCC_GPLL0_AO_OUT_MAIN 123
+    apcs: mailbox@b011000 {
+        compatible = "qcom,qcs404-apcs-apps-global";
+        reg = <0x0b011000 0x1000>;
+        #mbox-cells = <1>;
+        clocks = <&apcs_hfpll>, <&gcc GCC_GPLL0_AO_OUT_MAIN>;
+        clock-names = "pll", "aux";
+        #clock-cells = <0>;
+    };
-- 
2.7.4


^ permalink raw reply related

* [PATCH 4/4] arm64: dts: ipq6018: Add support for apcs clk
From: Sivaprakash Murugesan @ 2020-05-27 12:38 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, jassisinghbrar, linux-arm-msm,
	devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590583092-24290-1-git-send-email-sivaprak@codeaurora.org>

The ipq6018 devices has a clock functionality in apcs blcok. Add support
for the clock found in ipq6018 apcs block.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 * This patch has compilation dependency with apss pll
	https://lkml.org/lkml/2020/5/27/642
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 3956e44..8d60f6f 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -294,9 +294,11 @@
 		};
 
 		apcs_glb: mailbox@b111000 {
-			compatible = "qcom,ipq8074-apcs-apps-global";
-			reg = <0x0b111000 0xc>;
-
+			compatible = "qcom,ipq6018-apcs-apps-global";
+			reg = <0x0b111000 0x1000>;
+			#clock-cells = <1>;
+			clocks = <&apsspll>, <&xo>;
+			clock-names = "pll", "xo";
 			#mbox-cells = <1>;
 		};
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 3/4] mailbox: qcom: Add ipq6018 apcs compatible
From: Sivaprakash Murugesan @ 2020-05-27 12:38 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, jassisinghbrar, linux-arm-msm,
	devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590583092-24290-1-git-send-email-sivaprak@codeaurora.org>

The Qualcomm ipq6018 has apcs block, add compatible for the same.
Also, the apcs provides a clock controller functionality similar
to msm8916 but the clock driver is different.

Create a child platform device based on the apcs compatible for the
clock controller functionality.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 drivers/mailbox/qcom-apcs-ipc-mailbox.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index eeebafd..db3f9518 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -45,6 +45,13 @@ static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
 	.send_data = qcom_apcs_ipc_send_data,
 };
 
+static const struct of_device_id apcs_clk_match_table[] = {
+	{ .compatible = "qcom,ipq6018-apcs-apps-global", .data = "qcom,apss-ipq6018-clk", },
+	{ .compatible = "qcom,msm8916-apcs-kpss-global", .data = "qcom-apcs-msm8916-clk", },
+	{ .compatible = "qcom,qcs404-apcs-apps-global",  .data = "qcom-apcs-msm8916-clk", },
+	{}
+};
+
 static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 {
 	struct qcom_apcs_ipc *apcs;
@@ -54,11 +61,7 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 	void __iomem *base;
 	unsigned long i;
 	int ret;
-	const struct of_device_id apcs_clk_match_table[] = {
-		{ .compatible = "qcom,msm8916-apcs-kpss-global", },
-		{ .compatible = "qcom,qcs404-apcs-apps-global", },
-		{}
-	};
+	const struct of_device_id *clk_device;
 
 	apcs = devm_kzalloc(&pdev->dev, sizeof(*apcs), GFP_KERNEL);
 	if (!apcs)
@@ -93,11 +96,12 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (of_match_device(apcs_clk_match_table, &pdev->dev)) {
+	clk_device = of_match_device(apcs_clk_match_table, &pdev->dev);
+	if (clk_device) {
 		apcs->clk = platform_device_register_data(&pdev->dev,
-							  "qcom-apcs-msm8916-clk",
-							  PLATFORM_DEVID_NONE,
-							  NULL, 0);
+							clk_device->data,
+							PLATFORM_DEVID_NONE,
+							NULL, 0);
 		if (IS_ERR(apcs->clk))
 			dev_err(&pdev->dev, "failed to register APCS clk\n");
 	}
@@ -126,6 +130,7 @@ static const struct of_device_id qcom_apcs_ipc_of_match[] = {
 	{ .compatible = "qcom,sc7180-apss-shared", .data = (void *)12 },
 	{ .compatible = "qcom,sdm845-apss-shared", .data = (void *)12 },
 	{ .compatible = "qcom,sm8150-apss-shared", .data = (void *)12 },
+	{ .compatible = "qcom,ipq6018-apcs-apps-global", .data = (void *)8 },
 	{ .compatible = "qcom,ipq8074-apcs-apps-global", .data = (void *)8 },
 	{}
 };
-- 
2.7.4


^ permalink raw reply related

* [PATCH 0/4] Add ipq6018 apcs mailbox driver
From: Sivaprakash Murugesan @ 2020-05-27 12:38 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, jassisinghbrar, linux-arm-msm,
	devicetree, linux-kernel
  Cc: Sivaprakash Murugesan

The ipq6018 devices has apcs block for ipc interrupts, this block also
provides a clock controller which provides cpu clocks.

This series adds support for the apcs block found in ipq6018 devices.

This series was originally part of ipq6018 apss clock controller series
https://lkml.org/lkml/2020/5/27/637

The patch 4 has dtb dependency with apss pll driver which can be found in
https://lkml.org/lkml/2020/5/27/637

Sivaprakash Murugesan (4):
  dt-bindings: mailbox: Add YAML schemas for QCOM APCS global block
  dt-bindings: mailbox: Add dt-bindings for ipq6018 apcs global block
  mailbox: qcom: Add ipq6018 apcs compatible
  arm64: dts: ipq6018: Add support for apcs clk

 .../bindings/mailbox/qcom,apcs-kpss-global.txt     | 88 -------------------
 .../bindings/mailbox/qcom,apcs-kpss-global.yaml    | 99 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/ipq6018.dtsi              |  8 +-
 drivers/mailbox/qcom-apcs-ipc-mailbox.c            | 23 +++--
 4 files changed, 118 insertions(+), 100 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.txt
 create mode 100644 Documentation/devicetree/bindings/mailbox/qcom,apcs-kpss-global.yaml

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH v5 06/11] net: ethernet: mtk-star-emac: new driver
From: Arnd Bergmann @ 2020-05-27 12:28 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Nathan Chancellor, Rob Herring, David S . Miller,
	Matthias Brugger, John Crispin, Sean Wang, Mark Lee,
	Jakub Kicinski, Fabien Parent, Heiner Kallweit, Edwin Peer,
	devicetree, Linux Kernel Mailing List, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Stephane Le Provost,
	Pedro Tsai, Andrew Perepech, Bartosz Golaszewski,
	clang-built-linux
In-Reply-To: <CAMRc=Md1w_6+dU9gCwiiB5R+dMcYMPFLPrA++RBkKp5zaY6Riw@mail.gmail.com>

On Wed, May 27, 2020 at 1:49 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> śr., 27 maj 2020 o 13:33 Arnd Bergmann <arnd@arndb.de> napisał(a):
> >
> > On Wed, May 27, 2020 at 10:46 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > Thanks for reporting this! I have a fix ready and will send it shortly.
> >
> > I already have a workaround for this bug as well as another one
> > in my tree that I'll send later today after some more testing.
> >
> > Feel free to wait for that, or just ignore mine if you already have a fix.
> >
>
> I already posted a fix[1]. Sorry for omitting you, but somehow your
> name didn't pop up in get_maintainers.pl.

I'm not a maintainer for this, I just do a lot of build fixes on the side,
as I verify the stuff that I merge myself ;-)

> [1] https://lkml.org/lkml/2020/5/27/378

Ok, perfect, that is indeed the correct fix and mine was wrong. I'll
just send a fix for the other bug (unused-function warning) then.

     Arnd

^ permalink raw reply

* [PATCH] check: Add 10bit/slave i2c reg flags support
From: Serge Semin @ 2020-05-27 12:25 UTC (permalink / raw)
  To: devicetree-compiler
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
	Wolfram Sang, Jarkko Nikula, Andy Shevchenko, Frank Rowand,
	Rob Herring, devicetree, linux-i2c, linux-kernel

Recently the I2C-controllers slave interface support was added to the
kernel I2C subsystem. In this case Linux can be used as, for example,
a I2C EEPROM machine. See [1] for details. Other than instantiating
the EEPROM-slave device from user-space there is a way to declare the
device in dts. In this case firstly the I2C bus controller must support
the slave interface. Secondly I2C-slave sub-node of that controller
must have "reg"-property with flag I2C_OWN_SLAVE_ADDRESS set (flag is
declared in [2]). That flag is declared as (1 << 30), which when set
makes dtc unhappy about too big address set for a I2C-slave:

Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"

Similar problem would have happened if we had set the 10-bit address
flag I2C_TEN_BIT_ADDRESS in the "reg"-property.

In order to fix the problem we suggest to alter the I2C-bus reg-check
algorithm, so one would be aware of the upper bits set. Normally if no
flag specified, the 7-bit address is expected in the "reg"-property.
If I2C_TEN_BIT_ADDRESS is set, then the 10-bit address check will be
performed. The I2C_OWN_SLAVE_ADDRESS flag will be just ignored.

[1] kernel/Documentation/i2c/slave-interface.rst
[2] kernel/include/dt-bindings/i2c/i2c.h

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree-compiler@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 checks.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/checks.c b/checks.c
index 4b3c486f1399..6321fc5b7404 100644
--- a/checks.c
+++ b/checks.c
@@ -1028,6 +1028,7 @@ static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node
 	const char *unitname = get_unitname(node);
 	char unit_addr[17];
 	uint32_t reg = 0;
+	uint32_t addr;
 	int len;
 	cell_t *cells = NULL;
 
@@ -1044,17 +1045,21 @@ static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node
 	}
 
 	reg = fdt32_to_cpu(*cells);
-	snprintf(unit_addr, sizeof(unit_addr), "%x", reg);
+	addr = reg & 0x3FFFFFFFU;
+	snprintf(unit_addr, sizeof(unit_addr), "%x", addr);
 	if (!streq(unitname, unit_addr))
 		FAIL(c, dti, node, "I2C bus unit address format error, expected \"%s\"",
 		     unit_addr);
 
 	for (len = prop->val.len; len > 0; len -= 4) {
 		reg = fdt32_to_cpu(*(cells++));
-		if (reg > 0x3ff)
+		addr = reg & 0x3FFFFFFFU;
+		if ((reg & (1 << 31)) && addr > 0x3ff)
 			FAIL_PROP(c, dti, node, prop, "I2C address must be less than 10-bits, got \"0x%x\"",
-				  reg);
-
+				  addr);
+		else if (!(reg & (1 << 31)) && addr > 0x7f)
+			FAIL_PROP(c, dti, node, prop, "I2C address must be less than 7-bits, got \"0x%x\"",
+				  addr);
 	}
 }
 WARNING(i2c_bus_reg, check_i2c_bus_reg, NULL, &reg_format, &i2c_bus_bridge);
-- 
2.26.2


^ permalink raw reply related

* [PATCH V6 0/5] Add APSS clock controller support for IPQ6018
From: Sivaprakash Murugesan @ 2020-05-27 12:24 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel
  Cc: Sivaprakash Murugesan

The CPU on Qualcomm's IPQ6018 devices are primarily fed by APSS PLL and XO,
these are connected to a clock mux and enable block.

This patch series adds support for these clocks and inturn enables clocks
required for CPU freq.

[V6]
 * Split mailbox driver from this series, mailbox changes will sent as a
   separate series
 * Addressed review comments from Stephen
[V5]
 * Addressed Bjorn comments on apss clk and dt-bindings
 * Patch 2 depends on a53 pll dt-bindings
   https://www.spinics.net/lists/linux-clk/msg48358.html  
[V4]
 * Re-written PLL found on IPQ platforms as a separate driver
 * Addressed stephen's comments on apss clock controller and pll
 * Addressed Rob's review comments on bindings
 * moved a53 pll binding from this series as it is not applicable, will send
   it separately.
[V3]
 * Fixed dt binding check error in patch2
   dt-bindings: clock: Add YAML schemas for QCOM A53 PLL
[V2]
 * Restructred the patch series as there are two different HW blocks,
   the mux and enable belongs to the apcs block and PLL has a separate HW
   block.
 * Converted qcom mailbox and qcom a53 pll documentation to yaml.
 * Addressed review comments from Stephen, Rob and Sibi where it is applicable.
 * Changed this cover letter to state the purpose of this patch series

Sivaprakash Murugesan (5):
  dt-bindings: clock: add ipq6018 a53 pll compatible
  clk: qcom: Add ipq apss pll driver
  clk: qcom: Add DT bindings for ipq6018 apss clock controller
  clk: qcom: Add ipq6018 apss clock controller
  arm64: dts: ipq6018: Add support for apss pll

 .../devicetree/bindings/clock/qcom,a53pll.yaml     |  18 ++++
 arch/arm64/boot/dts/qcom/ipq6018.dtsi              |   8 ++
 drivers/clk/qcom/Kconfig                           |  19 ++++
 drivers/clk/qcom/Makefile                          |   2 +
 drivers/clk/qcom/apss-ipq-pll.c                    |  95 ++++++++++++++++++
 drivers/clk/qcom/apss-ipq6018.c                    | 106 +++++++++++++++++++++
 include/dt-bindings/clock/qcom,apss-ipq.h          |  12 +++
 7 files changed, 260 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq-pll.c
 create mode 100644 drivers/clk/qcom/apss-ipq6018.c
 create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h

-- 
2.7.4


^ permalink raw reply

* [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller
From: Sivaprakash Murugesan @ 2020-05-27 12:24 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590582292-13314-1-git-send-email-sivaprak@codeaurora.org>

The CPU on Qualcomm ipq6018 devices are clocked primarily by a aplha PLL
and xo which are connected to a mux and enable block.

Add support for the mux and enable block which feeds the CPU on ipq6018
devices.

Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 drivers/clk/qcom/Kconfig        |  11 +++++
 drivers/clk/qcom/Makefile       |   1 +
 drivers/clk/qcom/apss-ipq6018.c | 106 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq6018.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index e70aa01..b543e63 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -97,6 +97,17 @@ config IPQ_APSS_PLL
 	  Say Y if you want to support CPU frequency scaling on ipq based
 	  devices.
 
+config IPQ_APSS_6018
+	tristate "IPQ APSS Clock Controller"
+	select IPQ_APSS_PLL
+	depends on QCOM_APCS_IPC || COMPILE_TEST
+	help
+	  Support for APSS clock controller on IPQ platforms. The
+	  APSS clock controller manages the Mux and enable block that feeds the
+	  CPUs.
+	  Say Y if you want to support CPU frequency scaling on
+	  ipq based devices.
+
 config IPQ_GCC_4019
 	tristate "IPQ4019 Global Clock Controller"
 	help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index b4a6ba1..3accea1 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -20,6 +20,7 @@ clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
 obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
 obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
 obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
+obj-$(CONFIG_IPQ_APSS_6018) += apss-ipq6018.o
 obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
 obj-$(CONFIG_IPQ_GCC_6018) += gcc-ipq6018.o
 obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
new file mode 100644
index 0000000..004f7e1
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq6018.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+
+#include <dt-bindings/clock/qcom,apss-ipq.h>
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-branch.h"
+#include "clk-alpha-pll.h"
+#include "clk-regmap-mux.h"
+
+enum {
+	P_XO,
+	P_APSS_PLL_EARLY,
+};
+
+static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
+	{ .fw_name = "xo" },
+	{ .fw_name = "pll" },
+};
+
+static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
+	{ P_XO, 0 },
+	{ P_APSS_PLL_EARLY, 5 },
+};
+
+static struct clk_regmap_mux apcs_alias0_clk_src = {
+	.reg = 0x0050,
+	.width = 3,
+	.shift = 7,
+	.parent_map = parents_apcs_alias0_clk_src_map,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "apcs_alias0_clk_src",
+		.parent_data = parents_apcs_alias0_clk_src,
+		.num_parents = 2,
+		.ops = &clk_regmap_mux_closest_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_branch apcs_alias0_core_clk = {
+	.halt_reg = 0x0058,
+	.clkr = {
+		.enable_reg = 0x0058,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "apcs_alias0_core_clk",
+			.parent_hws = (const struct clk_hw *[]){
+				&apcs_alias0_clk_src.clkr.hw },
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_ops,
+		},
+	},
+};
+
+static const struct regmap_config apss_ipq6018_regmap_config = {
+	.reg_bits       = 32,
+	.reg_stride     = 4,
+	.val_bits       = 32,
+	.max_register   = 0x1000,
+	.fast_io        = true,
+};
+
+static struct clk_regmap *apss_ipq6018_clks[] = {
+	[APCS_ALIAS0_CLK_SRC] = &apcs_alias0_clk_src.clkr,
+	[APCS_ALIAS0_CORE_CLK] = &apcs_alias0_core_clk.clkr,
+};
+
+static const struct qcom_cc_desc apss_ipq6018_desc = {
+	.config = &apss_ipq6018_regmap_config,
+	.clks = apss_ipq6018_clks,
+	.num_clks = ARRAY_SIZE(apss_ipq6018_clks),
+};
+
+static int apss_ipq6018_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+
+	regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return qcom_cc_really_probe(pdev, &apss_ipq6018_desc, regmap);
+}
+
+static struct platform_driver apss_ipq6018_driver = {
+	.probe = apss_ipq6018_probe,
+	.driver = {
+		.name   = "qcom,apss-ipq6018-clk",
+	},
+};
+
+module_platform_driver(apss_ipq6018_driver);
+
+MODULE_DESCRIPTION("QCOM APSS IPQ 6018 CLK Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related

* [PATCH V6 5/5] arm64: dts: ipq6018: Add support for apss pll
From: Sivaprakash Murugesan @ 2020-05-27 12:24 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590582292-13314-1-git-send-email-sivaprak@codeaurora.org>

Enable apss pll support.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V6]
 * split the mailbox driver from this patch
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 1aa8d85..3956e44 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -300,6 +300,14 @@
 			#mbox-cells = <1>;
 		};
 
+		apsspll: clock@b116000 {
+			compatible = "qcom,ipq6018-a53pll";
+			reg = <0x0b116000 0x40>;
+			#clock-cells = <0>;
+			clocks = <&xo>;
+			clock-names = "xo";
+		};
+
 		timer {
 			compatible = "arm,armv8-timer";
 			interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
-- 
2.7.4


^ permalink raw reply related

* [PATCH V6 3/5] clk: qcom: Add DT bindings for ipq6018 apss clock controller
From: Sivaprakash Murugesan @ 2020-05-27 12:24 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590582292-13314-1-git-send-email-sivaprak@codeaurora.org>

Add dt-binding for ipq6018 apss clock controller

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V6]
 * Addressed review comment from Stephen
 include/dt-bindings/clock/qcom,apss-ipq.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h

diff --git a/include/dt-bindings/clock/qcom,apss-ipq.h b/include/dt-bindings/clock/qcom,apss-ipq.h
new file mode 100644
index 0000000..77b6e05
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,apss-ipq.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
+#define _DT_BINDINGS_CLOCK_QCA_APSS_IPQ6018_H
+
+#define APCS_ALIAS0_CLK_SRC			0
+#define APCS_ALIAS0_CORE_CLK			1
+
+#endif
-- 
2.7.4


^ permalink raw reply related

* [PATCH V6 1/5] dt-bindings: clock: add ipq6018 a53 pll compatible
From: Sivaprakash Murugesan @ 2020-05-27 12:24 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590582292-13314-1-git-send-email-sivaprak@codeaurora.org>

cpus on ipq6018 are clocked by a53 pll, add device compatible for a53
pll found on ipq6018 devices.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
* [V6]
    re-ordered compatible string, dropped Rob's review tag for this change.
 .../devicetree/bindings/clock/qcom,a53pll.yaml         | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
index 20d2638..a4f2d01 100644
--- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
@@ -15,6 +15,7 @@ description:
 
 properties:
   compatible:
+    const: qcom,ipq6018-a53pll
     const: qcom,msm8916-a53pll
 
   reg:
@@ -23,6 +24,14 @@ properties:
   '#clock-cells':
     const: 0
 
+  clocks:
+    items:
+      - description: board XO clock
+
+  clock-names:
+    items:
+      - const: xo
+
 required:
   - compatible
   - reg
@@ -38,3 +47,12 @@ examples:
         reg = <0xb016000 0x40>;
         #clock-cells = <0>;
     };
+  #Example 2 - A53 PLL found on IPQ6018 devices
+  - |
+    a53pll_ipq: clock@b116000 {
+        compatible = "qcom,ipq6018-a53pll";
+        reg = <0x0b116000 0x40>;
+        #clock-cells = <0>;
+        clocks = <&xo>;
+        clock-names = "xo";
+    };
-- 
2.7.4


^ permalink raw reply related

* [PATCH V6 2/5] clk: qcom: Add ipq apss pll driver
From: Sivaprakash Murugesan @ 2020-05-27 12:24 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, robh+dt,
	linux-arm-msm, linux-clk, devicetree, linux-kernel
  Cc: Sivaprakash Murugesan
In-Reply-To: <1590582292-13314-1-git-send-email-sivaprak@codeaurora.org>

The CPUs on Qualcomm ipq based devices are clocked by an alpha PLL.
Add support for the apss pll found on ipq based devices which can
support CPU frequencies above 1Ghz.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
[V6]
 * Addressed review comments from Stephen
 drivers/clk/qcom/Kconfig        |  8 ++++
 drivers/clk/qcom/Makefile       |  1 +
 drivers/clk/qcom/apss-ipq-pll.c | 95 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)
 create mode 100644 drivers/clk/qcom/apss-ipq-pll.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 11ec6f4..e70aa01 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -89,6 +89,14 @@ config APQ_MMCC_8084
 	  Say Y if you want to support multimedia devices such as display,
 	  graphics, video encode/decode, camera, etc.
 
+config IPQ_APSS_PLL
+	tristate "IPQ APSS PLL"
+	help
+	  Support for APSS PLL on ipq devices. The APSS PLL is the main
+	  clock that feeds the CPUs on ipq based devices.
+	  Say Y if you want to support CPU frequency scaling on ipq based
+	  devices.
+
 config IPQ_GCC_4019
 	tristate "IPQ4019 Global Clock Controller"
 	help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 691efbf..b4a6ba1 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -19,6 +19,7 @@ clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
 # Keep alphabetically sorted by config
 obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
 obj-$(CONFIG_APQ_MMCC_8084) += mmcc-apq8084.o
+obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
 obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
 obj-$(CONFIG_IPQ_GCC_6018) += gcc-ipq6018.o
 obj-$(CONFIG_IPQ_GCC_806X) += gcc-ipq806x.o
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
new file mode 100644
index 0000000..e34f4cd
--- /dev/null
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018, The Linux Foundation. All rights reserved.
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "clk-alpha-pll.h"
+
+static const u8 ipq_pll_offsets[] = {
+	[PLL_OFF_L_VAL] = 0x08,
+	[PLL_OFF_ALPHA_VAL] = 0x10,
+	[PLL_OFF_USER_CTL] = 0x18,
+	[PLL_OFF_CONFIG_CTL] = 0x20,
+	[PLL_OFF_CONFIG_CTL_U] = 0x24,
+	[PLL_OFF_STATUS] = 0x28,
+	[PLL_OFF_TEST_CTL] = 0x30,
+	[PLL_OFF_TEST_CTL_U] = 0x34,
+};
+
+static struct clk_alpha_pll ipq_pll = {
+	.offset = 0x0,
+	.regs = ipq_pll_offsets,
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x0,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "a53pll",
+			.parent_data = &(const struct clk_parent_data) {
+				.fw_name = "xo",
+			},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_huayra_ops,
+		},
+	},
+};
+
+static const struct alpha_pll_config ipq_pll_config = {
+	.l = 0x37,
+	.config_ctl_val = 0x04141200,
+	.config_ctl_hi_val = 0x0,
+	.early_output_mask = BIT(3),
+	.main_output_mask = BIT(0),
+};
+
+static const struct regmap_config ipq_pll_regmap_config = {
+	.reg_bits		= 32,
+	.reg_stride		= 4,
+	.val_bits		= 32,
+	.max_register		= 0x40,
+	.fast_io		= true,
+};
+
+static int apss_ipq_pll_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct regmap *regmap;
+	void __iomem *base;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio(dev, base, &ipq_pll_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	clk_alpha_pll_configure(&ipq_pll, regmap, &ipq_pll_config);
+
+	ret = devm_clk_register_regmap(dev, &ipq_pll.clkr);
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+							&ipq_pll.clkr.hw);
+}
+
+static const struct of_device_id apss_ipq_pll_match_table[] = {
+	{ .compatible = "qcom,ipq6018-a53pll" },
+	{ }
+};
+
+static struct platform_driver apss_ipq_pll_driver = {
+	.probe = apss_ipq_pll_probe,
+	.driver = {
+		.name = "qcom-ipq-apss-pll",
+		.of_match_table = apss_ipq_pll_match_table,
+	},
+};
+module_platform_driver(apss_ipq_pll_driver);
+
+MODULE_DESCRIPTION("Qualcomm technology Inc APSS ALPHA PLL Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH net-next v3 4/4] net: dp83869: Add RGMII internal delay configuration
From: Dan Murphy @ 2020-05-27 12:23 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200527005224.GF782807@lunn.ch>

Andrew

On 5/26/20 7:52 PM, Andrew Lunn wrote:
>> @@ -218,6 +224,7 @@ static int dp83869_of_init(struct phy_device *phydev)
>>   		ret = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1);
>>   		if (ret < 0)
>>   			return ret;
>> +
>>   		if (ret & DP83869_STRAP_MIRROR_ENABLED)
>>   			dp83869->port_mirroring = DP83869_PORT_MIRRORING_EN;
>>   		else
> This random white space change does not belong in this patch.

OK


>> @@ -232,6 +239,20 @@ static int dp83869_of_init(struct phy_device *phydev)
>>   				 &dp83869->tx_fifo_depth))
>>   		dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
>>   
>> +	ret = of_property_read_u32(of_node, "rx-internal-delay-ps",
>> +				   &dp83869->rx_id_delay);
>> +	if (ret) {
>> +		dp83869->rx_id_delay = ret;
>> +		ret = 0;
>> +	}
> This looks odd.
>
> If this optional property is not found, -EINVAL will be returned. It
> could also return -ENODATA. You then assign this error value to
> dp83869->rx_id_delay? I would of expected you to assign 2000, the
> default value?

Well the driver cannot assume this is the intended value.

If the dt defines rgmii-rx/tx-id then these values are required not 
optional.  That was the discussion on the binding.

I set these to errno because when config_init is called the driver 
verifies that the values are valid and present and if they

are not then the PHY will fail to init.

If we set the delay to default then the PHY may be programmed with the 
wrong delay.


>> +
>> +	ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
>> +				   &dp83869->tx_id_delay);
>> +	if (ret) {
>> +		dp83869->tx_id_delay = ret;
>> +		ret = 0;
>> +	}
>> +
>>   	return ret;
>>   }
>>   #else
>> @@ -367,10 +388,45 @@ static int dp83869_configure_mode(struct phy_device *phydev,
>>   	return ret;
>>   }
>>   
>> +static int dp83869_get_delay(struct phy_device *phydev)
>> +{
>> +	struct dp83869_private *dp83869 = phydev->priv;
>> +	int delay_size = ARRAY_SIZE(dp83869_internal_delay);
>> +	int tx_delay = 0;
>> +	int rx_delay = 0;
>> +
>> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
>> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>> +		tx_delay = phy_get_delay_index(phydev,
>> +					       &dp83869_internal_delay[0],
>> +					       delay_size, dp83869->tx_id_delay,
>> +					       false);
>> +		if (tx_delay < 0) {
>> +			phydev_err(phydev, "Tx internal delay is invalid\n");
>> +			return tx_delay;
>> +		}
>> +	}
>> +
>> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
>> +	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>> +		rx_delay = phy_get_delay_index(phydev,
>> +					       &dp83869_internal_delay[0],
>> +					       delay_size, dp83869->rx_id_delay,
>> +					       false);
>> +		if (rx_delay < 0) {
>> +			phydev_err(phydev, "Rx internal delay is invalid\n");
>> +			return rx_delay;
>> +		}
>> +	}
> So any PHY using these properties is going to pretty much reproduce
> this code. Meaning is should all be in a helper.

The issue here is that the phy_mode may only be rgmii-txid so you only 
want to find the tx_delay and return.

Same with the RXID.  How is the helper supposed to know what delay to 
return and look for?

The PHY also only needs to use the helper if the PHY is in certain modes.

And the decision to use the checks is really based on the PHY driver.

Not sure if other PHYs delays require both delays to be set or if the 
delays are independent.

The helper cannot assume this.

Dan


>
>       Andrew

^ permalink raw reply

* Re: [PATCH net-next v3 2/4] net: phy: Add a helper to return the index for of the internal delay
From: Dan Murphy @ 2020-05-27 12:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200527004220.GE782807@lunn.ch>

Andrew

On 5/26/20 7:42 PM, Andrew Lunn wrote:
>> +/**
>> + * phy_get_delay_index - returns the index of the internal delay
>> + * @phydev: phy_device struct
>> + * @delay_values: array of delays the PHY supports
>> + * @size: the size of the delay array
>> + * @int_delay: the internal delay to be looked up
>> + * @descending: if the delay array is in descending order
>> + *
>> + * Returns the index within the array of internal delay passed in.
>> + * Return errno if the delay is invalid or cannot be found.
>> + */
>> +s32 phy_get_delay_index(struct phy_device *phydev, int *delay_values, int size,
>> +			int int_delay, bool descending)
>> +{
>> +	if (int_delay < 0)
>> +		return -EINVAL;
>> +
>> +	if (size <= 0)
>> +		return -EINVAL;
>> +
>> +	if (descending)
>> +		return phy_find_descending_delay(phydev, delay_values, size,
>> +						 int_delay);
>> +
>> +	return phy_find_ascending_delay(phydev, delay_values, size, int_delay);
>> +}
>> +EXPORT_SYMBOL(phy_get_delay_index);
> Do we really need this ascending vs descending? This array is not
> coming from device tree of anything, it is a static list in the PHY
> driver. I would just define it needs to be ascending and be done.

I was thinking about the constraints of having just an ascending array 
helper.

If there is a PHY out there that has a descending delay array then this 
function is not a helper.

Then the PHY driver now has to implement a descending search or extend 
out this helper to do the same.

I can just keep it ascending for now but this helper may need to be 
updated in the future to accommodate any PHYs with descending delay arrays.

Dan


> 	Andrew

^ 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