Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v10 2/4] dtc: Document the dynamic plugin internals
From: Frank Rowand @ 2016-11-29  1:36 UTC (permalink / raw)
  To: Pantelis Antoniou, David Gibson
  Cc: Jon Loeliger, Grant Likely, Rob Herring, Jan Luebbe, Sascha Hauer,
	Phil Elwell, Simon Glass, Maxime Ripard, Thomas Petazzoni,
	Boris Brezillon, Antoine Tenart, Stephen Boyd,
	Devicetree Compiler, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480077131-14526-3-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

On 11/25/16 04:32, Pantelis Antoniou wrote:
> Provides the document explaining the internal mechanics of
> plugins and options.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/dt-object-internal.txt | 318 +++++++++++++++++++++++++++++++++++
>  1 file changed, 318 insertions(+)
>  create mode 100644 Documentation/dt-object-internal.txt
> 
> diff --git a/Documentation/dt-object-internal.txt b/Documentation/dt-object-internal.txt
> new file mode 100644
> index 0000000..d5b841e
> --- /dev/null
> +++ b/Documentation/dt-object-internal.txt
> @@ -0,0 +1,318 @@
> +Device Tree Dynamic Object format internals
> +-------------------------------------------
> +
> +The Device Tree for most platforms is a static representation of
> +the hardware capabilities. This is insufficient for many platforms
> +that need to dynamically insert device tree fragments to the
> +running kernel's live tree.
> +
> +This document explains the the device tree object format and the
> +modifications made to the device tree compiler, which make it possible.
> +
> +1. Simplified Problem Definition
> +--------------------------------
> +
> +Assume we have a platform which boots using following simplified device tree.
> +
> +---- foo.dts -----------------------------------------------------------------
> +	/* FOO platform */
> +	/ {
> +		compatible = "corp,foo";
> +
> +		/* shared resources */
> +		res: res {
> +		};
> +
> +		/* On chip peripherals */
> +		ocp: ocp {
> +			/* peripherals that are always instantiated */
> +			peripheral1 { ... };
> +		};
> +	};
> +---- foo.dts -----------------------------------------------------------------
> +
> +We have a number of peripherals that after probing (using some undefined method)
> +should result in different device tree configuration.
> +
> +We cannot boot with this static tree because due to the configuration of the
> +foo platform there exist multiple conficting peripherals DT fragments.

                                     ^^^^^^^^^^  conflicting

I assume conflicting because, for instance, the different peripherals might
occupy the same address space, use the same interrupt, or use the same gpio.
Mentioning that would provide a fuller picture for the neophyte.

> +
> +So for the bar peripheral we would have this:
> +
> +---- foo+bar.dts -------------------------------------------------------------
> +	/* FOO platform + bar peripheral */
> +	/ {
> +		compatible = "corp,foo";
> +
> +		/* shared resources */
> +		res: res {
> +		};
> +
> +		/* On chip peripherals */
> +		ocp: ocp {
> +			/* peripherals that are always instantiated */
> +			peripheral1 { ... };
> +
> +			/* bar peripheral */
> +			bar {
> +				compatible = "corp,bar";
> +				... /* various properties and child nodes */
> +			};
> +		};
> +	};
> +---- foo+bar.dts -------------------------------------------------------------
> +
> +While for the baz peripheral we would have this:
> +
> +---- foo+baz.dts -------------------------------------------------------------
> +	/* FOO platform + baz peripheral */
> +	/ {
> +		compatible = "corp,foo";
> +
> +		/* shared resources */
> +		res: res {
> +			/* baz resources */
> +			baz_res: res_baz { ... };
> +		};
> +
> +		/* On chip peripherals */
> +		ocp: ocp {
> +			/* peripherals that are always instantiated */
> +			peripheral1 { ... };
> +
> +			/* baz peripheral */
> +			baz {
> +				compatible = "corp,baz";
> +				/* reference to another point in the tree */
> +				ref-to-res = <&baz_res>;
> +				... /* various properties and child nodes */
> +			};
> +		};
> +	};
> +---- foo+baz.dts -------------------------------------------------------------
> +
> +We note that the baz case is more complicated, since the baz peripheral needs to
> +reference another node in the DT tree.

I know that there are other situations that can justify overlays, so not
contesting the basic need with this comment.  But the above situation could
be handled in a much simpler fashion by setting the status property of each
of the conflicting devices to disabled, then after probing setting the status
to ok.  That method removes a lot of complexity.

A big driver for the concept of overlays was being able to describe different
add on boards at run time, instead of when the base dtb was created.  I think
we have agreed that moving to a connector model instead of a raw overlay is
the proper way to address add on boards.

Can you address how an overlay can be created that will work for a board
plugged into any of the identical sockets that is compatible with the
board?


> +
> +2. Device Tree Object Format Requirements
> +-----------------------------------------
> +
> +Since the device tree is used for booting a number of very different hardware
> +platforms it is imperative that we tread very carefully.
> +
> +2.a) No changes to the Device Tree binary format for the base tree. We cannot
> +modify the tree format at all and all the information we require should be
> +encoded using device tree itself. We can add nodes that can be safely ignored
> +by both bootloaders and the kernel. The plugin dtb's are optionally tagged
> +with a different magic number in the header but otherwise they too are simple
> +blobs.
> +
> +2.b) Changes to the DTS source format should be absolutely minimal, and should
> +only be needed for the DT fragment definitions, and not the base boot DT.
> +
> +2.c) An explicit option should be used to instruct DTC to generate the required
> +information needed for object resolution. Platforms that don't use the
> +dynamic object format can safely ignore it.
> +
> +2.d) Finally, DT syntax changes should be kept to a minimum. It should be
> +possible to express everything using the existing DT syntax.
> +
> +3. Implementation
> +-----------------
> +
> +The basic unit of addressing in Device Tree is the phandle. Turns out it's
> +relatively simple to extend the way phandles are generated and referenced
> +so that it's possible to dynamically convert symbolic references (labels)
> +to phandle values. This is a valid assumption as long as the author uses
> +reference syntax and does not assign phandle values manually (which might
> +be a problem with decompiled source files).
> +
> +We can roughly divide the operation into two steps.
> +
> +3.a) Compilation of the base board DTS file using the '-@' option
> +generates a valid DT blob with an added __symbols__ node at the root node,
> +containing a list of all nodes that are marked with a label.
> +
> +Using the foo.dts file above the following node will be generated;
> +
> +$ dtc -@ -O dtb -o foo.dtb -b 0 foo.dts
> +$ fdtdump foo.dtb
> +...
> +/ {
> +	...
> +	res {
> +		...
> +		phandle = <0x00000001>;
> +		...
> +	};
> +	ocp {
> +		...
> +		phandle = <0x00000002>;
> +		...
> +	};
> +	__symbols__ {
> +		res="/res";
> +		ocp="/ocp";
> +	};
> +};
> +
> +Notice that all the nodes that had a label have been recorded, and that
> +phandles have been generated for them.
> +
> +This blob can be used to boot the board normally, the __symbols__ node will
> +be safely ignored both by the bootloader and the kernel (the only loss will
> +be a few bytes of memory and disk space).
> +
> +3.b) The Device Tree fragments must be compiled with the same option but they
> +must also have a tag (/plugin/) that allows undefined references to nodes
> +that are not present at compilation time to be recorded so that the runtime
> +loader can fix them.
> +
> +So the bar peripheral's DTS format would be of the form:
> +
> +/dts-v1/ /plugin/;	/* allow undefined references and record them */
> +/ {
> +	....	/* various properties for loader use; i.e. part id etc. */
> +	fragment@0 {
> +		target = <&ocp>;
> +		__overlay__ {
> +			/* bar peripheral */
> +			bar {
> +				compatible = "corp,bar";
> +				... /* various properties and child nodes */
> +			}
> +		};
> +	};
> +};

The last version of your patches that I tested did not require specifying
the target property, the fragment node, and the __overlay__ node.  dtc
properly created all of those items automatically.  For example, I could
go to all of the trouble of creating those items in a dts like:

$ cat example_1_hand_coded.dts
/dts-v1/;
/plugin/;

/ {

	fragment@0 {
		target = <&am3353x_pinmux>;

		__overlay__ {

			i2c1_pins: pinmux_i2c1_pins {
				pinctrl-single,pins = <
					0x158 0x72
					0x15c 0x72
				>;
			};
		};
	};

	fragment@1 {
		target = <&i2c1>;

		__overlay__ {
			pinctrl-names = "default";
			pinctrl-0 = <&i2c1_pins>;
			clock-frequency = <400000>;
			status = "okay";

			at24@50 {
				compatible = "at,24c256";
				pagesize = <64>;
				reg = <0x50>;
			};
		};
	};
};


Or I could let dtc automagically create all the special features
(target, fragment, __overlay__) from an equivalent dts:

$ cat example_1.dts
/dts-v1/;
/plugin/;


		&am3353x_pinmux {
			i2c1_pins: pinmux_i2c1_pins {
				pinctrl-single,pins = <
					0x158 0x72
					0x15c 0x72
				>;
			};
		};

		&i2c1 {
			#address-cells = <1>;
			#size-cells = <0>;
			pinctrl-names = "default";
			pinctrl-0 = <&i2c1_pins>;
			clock-frequency = <400000>;
			status = "okay";

			at24@50 {
				compatible = "at,24c256";
				pagesize = <64>;
				reg = <0x50>;
			};
		};


I would much prefer that people never hand code the target, fragment, and
__overlay__ in a dts source file.  Exposing them at the source level adds
complexity, confusion, and an increased chance of creating an invalid
overlay dtb.

If possible, I would prefer target, fragment, and __overlay__ not be valid
input to dtc.  It would probably be difficult to prohibit target and fragment,
because however unlikely they are as property and node names, they are valid
dts syntax before adding the overlay enhancements to dtc.  However __overlay__
is not a valid node name without the overlay enhancements and could remain
invalid dts input.

I prefer that target, fragment, and __overlay__ be documented as a dtb to
target system API.  In this case, for the normal developer, they are
hidden in the binary dtb format and in the kernel (or boot loader)
overlay framework code.

I do recognize that if __overlay__ is not valid dtc input then it is not
possible to decompile an overlay into a dts containing __overlay__ and
then recompile that dts.  This could be resolved by a more complex
decompile that turned the overlay dtb back into the form of example_1.dts
above.

After reading to the end of this patch, I see that the simpler form of
.dts (like example_1.dts) is also noted as "an alternative syntax to
the expanded form for overlays".


> +
> +Note that there's a target property that specifies the location where the
> +contents of the overlay node will be placed, and it references the node
> +in the foo.dts file.
> +
> +$ dtc -@ -O dtb -o bar.dtbo -b 0 bar.dts
> +$ fdtdump bar.dtbo
> +...
> +/ {
> +	... /* properties */
> +	fragment@0 {
> +		target = <0xffffffff>;
> +		__overlay__ {
> +			bar {
> +				compatible = "corp,bar";
> +				... /* various properties and child nodes */
> +			}
> +		};
> +	};
> +	__fixups__ {
> +	    ocp = "/fragment@0:target:0";
> +	};
> +};
> +
> +No __symbols__ has been generated (no label in bar.dts).
> +Note that the target's ocp label is undefined, so the phandle handle
> +value is filled with the illegal value '0xffffffff', while a __fixups__
> +node has been generated, which marks the location in the tree where
> +the label lookup should store the runtime phandle value of the ocp node.
> +
> +The format of the __fixups__ node entry is
> +
> +	<label> = "<local-full-path>:<property-name>:<offset>";
> +
> +<label> 		Is the label we're referring
> +<local-full-path>	Is the full path of the node the reference is
> +<property-name>		Is the name of the property containing the
> +			reference
> +<offset>		The offset (in bytes) of where the property's
> +			phandle value is located.
> +
> +Doing the same with the baz peripheral's DTS format is a little bit more
> +involved, since baz contains references to local labels which require
> +local fixups.
> +
> +/dts-v1/ /plugin/;	/* allow undefined label references and record them */
> +/ {
> +	....	/* various properties for loader use; i.e. part id etc. */
> +	fragment@0 {
> +		target = <&res>;
> +		__overlay__ {
> +			/* baz resources */
> +			baz_res: res_baz { ... };
> +		};
> +	};
> +	fragment@1 {
> +		target = <&ocp>;
> +		__overlay__ {
> +			/* baz peripheral */
> +			baz {
> +				compatible = "corp,baz";
> +				/* reference to another point in the tree */
> +				ref-to-res = <&baz_res>;
> +				... /* various properties and child nodes */
> +			}
> +		};
> +	};
> +};
> +
> +Note that &bar_res reference.
> +
> +$ dtc -@ -O dtb -o baz.dtbo -b 0 baz.dts
> +$ fdtdump baz.dtbo
> +...
> +/ {
> +	... /* properties */
> +	fragment@0 {
> +		target = <0xffffffff>;
> +		__overlay__ {
> +			res_baz {
> +				....
> +				phandle = <0x00000001>;
> +			};
> +		};
> +	};
> +	fragment@1 {
> +		target = <0xffffffff>;
> +		__overlay__ {
> +			baz {
> +				compatible = "corp,baz";
> +				... /* various properties and child nodes */
> +				ref-to-res = <0x00000001>;
> +			}
> +		};
> +	};
> +	__fixups__ {
> +		res = "/fragment@0:target:0";
> +		ocp = "/fragment@1:target:0";
> +	};
> +	__local_fixups__ {
> +		fragment@1 {
> +			__overlay__ {
> +				baz {
> +					ref-to-res = <0>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +This is similar to the bar case, but the reference of a local label by the
> +baz node generates a __local_fixups__ entry that records the place that the
> +local reference is being made. No matter how phandles are allocated from dtc
> +the run time loader must apply an offset to each phandle in every dynamic
> +DT object loaded. The __local_fixups__ node records the place of every
> +local reference so that the loader can apply the offset.
> +
> +There is an alternative syntax to the expanded form for overlays with phandle
> +targets which makes the format similar to the one using in .dtsi include files.
> +
> +So for the &ocp target example above one can simply write:
> +
> +/dts-v1/ /plugin/;
> +&ocp {
> +	/* bar peripheral */
> +	bar {
> +		compatible = "corp,bar";
> +		... /* various properties and child nodes */
> +	}
> +};
> +
> +The resulting dtb object is identical.
> 

^ permalink raw reply

* Re: [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eduardo Valentin @ 2016-11-29  1:34 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Martin Sperl, Zhang Rui, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Lee Jones, Russell King, Florian Fainelli,
	Catalin Marinas, Will Deacon, linux-pm, devicetree,
	linux-rpi-kernel, linux-arm-kernel
In-Reply-To: <87twarz6s1.fsf@eliezer.anholt.net>

Hello Eric, Martin,

On Mon, Nov 28, 2016 at 12:30:38PM -0800, Eric Anholt wrote:
> Eduardo Valentin <edubezval@gmail.com> writes:
> 

<cut> 

> The firmware today always initializes thermal.  I suggested adding the
> init code because we (myself and the Pi Foundation) would like to reduce
> how much closed firmware code is required in the platform, and the Linux
> driver doing this would help make that possible in the future.

Oh I see. Backup code for future chips/firmware.

> 
> >> > Who has the ownership of this device?
> >> 
> >> Joined ownership I suppose...
> >> 
> >
> > with no synchronization mechanism?
> 
> Correct, because none is necessary.
> 



<cut>

> 
> Either the device was initialized by the firmware before handing off to
> ARM (today's firmware) or it never will be (potential future firmware).

And do you have a way to check if the firmware has the initialization
code or not? By firmware version, for example. Or even, chip version,
maybe?

If the current firmware will always initialize the chip, I would say,
ARM should simply read the registers, no initialization, unless it is
known that the firmware intentionally left the device uninitialized.

Again, just trying to avoid obscure misbehavior, when running into a
faulty state, and running silently broken.

^ permalink raw reply

* Re: [PATCH V2 09/10] thermal: da9062/61: Thermal junction temperature monitoring driver
From: Eduardo Valentin @ 2016-11-29  1:24 UTC (permalink / raw)
  To: Steve Twiss
  Cc: LINUX-KERNEL, LINUX-PM, Zhang Rui, DEVICETREE, Dmitry Torokhov,
	Guenter Roeck, LINUX-INPUT, LINUX-WATCHDOG, Lee Jones,
	Liam Girdwood, Mark Brown, Mark Rutland, Rob Herring,
	Support Opensource, Wim Van Sebroeck
In-Reply-To: <20fefc922818ab0511101f73b1a4d3468c9a8ed3.1477501000.git.stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>

Hello Steve,

On Wed, Oct 26, 2016 at 05:56:39PM +0100, Steve Twiss wrote:
> From: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> 
> Add junction temperature monitoring supervisor device driver, compatible
> with the DA9062 and DA9061 PMICs.
> 
> If the PMIC's internal junction temperature rises above TEMP_WARN (125
> degC) an interrupt is issued. This TEMP_WARN level is defined as the
> THERMAL_TRIP_HOT trip-wire inside the device driver.
> 
> The thermal triggering mechanism is interrupt based and happens when the
> temperature rises above a given threshold level. The component cannot
> return an exact temperature, it only has knowledge if the temperature is
> above or below a given threshold value. A status bit must be polled to
> detect when the temperature falls below that threshold level again. A
> kernel work queue is configured to repeatedly poll and detect when the
> temperature falls below this trip-wire, between 1 and 10 second intervals
> (defaulting at 3 seconds).
> 
> This first level of temperature supervision is intended for non-invasive
> temperature control, where the necessary measures for cooling the system
> down are left to the host software.
> 
> Signed-off-by: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> 
> ---
> This patch applies against linux-next and v4.8
> 
> v1 -> v2
>  - Patch renamed from [PATCH V1 05/10] to [PATCH V2 09/10] -- these
>    changes were made to fix checkpatch warnings caused by the patch
>    set dependency order
>  - List the header files in alphabetical order
>  - Remove "GPL v2" and replace with MODULE_LICENSE("GPL") to match the
>    copyright "GNU Public License v2 or later" option in the header
>    comment for this file. See the allowed identifiers in the file
>    include/linux/module.h +170
>  - Remove notify function "da9062_thermal_notify" function.
>  - MODULE_AUTHOR() macros removes Company Name and just gives Name in
>    accordance with include/linux/module.h +200
>  - Remove the compatible "dlg,da9061-thermal" option in the of_device_id
>    struct table. This patch now assumes the use of a DA9062 fallback
>    compatible string in the DTS when using the DA9061 thermal component
>    of the DA9061 device.
>  - Re-ordered some assignments earlier in the probe() for thermal->hw,
>    thermal->polling_period, thermal->mode, thermal->dev
>  - Added further information in the patch description to explain the use
>    of the device driver's built-in work-queue.
> 
> Regards,
> Steve Twiss, Dialog Semiconductor Ltd.
> 
> 
>  drivers/thermal/Kconfig          |  10 ++
>  drivers/thermal/Makefile         |   1 +
>  drivers/thermal/da9062-thermal.c | 289 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 300 insertions(+)
>  create mode 100644 drivers/thermal/da9062-thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 2d702ca..da58e54 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -272,6 +272,16 @@ config DB8500_CPUFREQ_COOLING
>  	  bound cpufreq cooling device turns active to set CPU frequency low to
>  	  cool down the CPU.
>  
> +config DA9062_THERMAL
> +	tristate "DA9062/DA9061 Dialog Semiconductor thermal driver"
> +	depends on MFD_DA9062
> +	depends on OF
> +	help
> +	  Enable this for the Dialog Semiconductor thermal sensor driver.
> +	  This will report PMIC junction over-temperature for one thermal trip
> +	  zone.
> +	  Compatible with the DA9062 and DA9061 PMICs.

Is there any hardware documentation available for this chip that can be
pointed out?

> +
>  config INTEL_POWERCLAMP
>  	tristate "Intel PowerClamp idle injection driver"
>  	depends on THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 10b07c1..0a2b3f2 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
>  obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
>  obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
>  obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
> +obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
>  obj-$(CONFIG_INTEL_POWERCLAMP)	+= intel_powerclamp.o
>  obj-$(CONFIG_X86_PKG_TEMP_THERMAL)	+= x86_pkg_temp_thermal.o
>  obj-$(CONFIG_INTEL_SOC_DTS_IOSF_CORE)	+= intel_soc_dts_iosf.o
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> new file mode 100644
> index 0000000..e0d2b1b
> --- /dev/null
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -0,0 +1,289 @@
> +/*
> + * Thermal device driver for DA9062 and DA9061
> + * Copyright (C) 2016  Dialog Semiconductor Ltd.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/thermal.h>
> +#include <linux/workqueue.h>
> +
> +#include <linux/mfd/da9062/core.h>
> +#include <linux/mfd/da9062/registers.h>
> +
> +#define DA9062_DEFAULT_POLLING_MS_PERIOD	3000
> +#define DA9062_MAX_POLLING_MS_PERIOD		10000
> +#define DA9062_MIN_POLLING_MS_PERIOD		1000
> +
> +#define DA9062_MILLI_CELSIUS(t)			((t)*1000)
> +
> +struct da9062_thermal_config {
> +	const char *name;
> +};
> +
> +struct da9062_thermal {
> +	struct da9062 *hw;
> +	struct delayed_work work;
> +	struct thermal_zone_device *zone;
> +	enum thermal_device_mode mode;
> +	unsigned int polling_period;
> +	struct mutex lock;
> +	int temperature;
> +	int irq;
> +	const struct da9062_thermal_config *config;
> +	struct device *dev;
> +};
> +
> +static void da9062_thermal_poll_on(struct work_struct *work)
> +{
> +	struct da9062_thermal *thermal = container_of(work,
> +						struct da9062_thermal,
> +						work.work);
> +	unsigned int val;
> +	int ret;
> +
> +	/* clear E_TEMP */
> +	ret = regmap_write(thermal->hw->regmap,
> +				DA9062AA_EVENT_B,
> +				DA9062AA_E_TEMP_MASK);
> +	if (ret < 0) {
> +		dev_err(thermal->dev,
> +			"Cannot clear the TJUNC temperature status\n");
> +		goto err_enable_irq;
> +	}
> +
> +	/* Now read E_TEMP again: it is acting like a status bit.
> +	 * If over-temperature, then this status will be true.
> +	 * If not over-temperature, this status will be false.
> +	 */
> +	ret = regmap_read(thermal->hw->regmap,
> +			  DA9062AA_EVENT_B,
> +			  &val);
> +	if (ret < 0) {
> +		dev_err(thermal->dev,
> +			"Cannot check the TJUNC temperature status\n");
> +		goto err_enable_irq;
> +	} else {
> +		if (val & DA9062AA_E_TEMP_MASK) {
> +			mutex_lock(&thermal->lock);
> +			thermal->temperature = DA9062_MILLI_CELSIUS(125);

Does this mean that the chip temperature is above or equal to 125C, is
this really a safe temperature to keep it running?

> +			mutex_unlock(&thermal->lock);
> +			thermal_zone_device_update(thermal->zone);
> +
> +			schedule_delayed_work(&thermal->work,
> +				msecs_to_jiffies(thermal->polling_period));
> +			return;
> +		} else {
> +			mutex_lock(&thermal->lock);
> +			thermal->temperature = DA9062_MILLI_CELSIUS(0);
> +			mutex_unlock(&thermal->lock);
> +			thermal_zone_device_update(thermal->zone);
> +		}
> +	}
> +
> +err_enable_irq:
> +	enable_irq(thermal->irq);
> +}
> +
> +static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> +{
> +	struct da9062_thermal *thermal = data;
> +
> +	disable_irq_nosync(thermal->irq);
> +	schedule_delayed_work(&thermal->work, 0);

Can you please elaborate a little on why you have an one shot threaded IRQ
handler that is only disabling the IRQ then, scheduling a work with delay of 0?

To my understanding, this is exactly what you get when you run your
threaded IRQ handler, when you configure it as one shot.

Have you tried simply handling the same work done in your workqueue
handler in your threaded IRQ? That should simplify your code and get the
same result you are expecting.

If you are not getting the IRQ disabled on the threaded IRQ when
configured as one shot, something else seams to be broken.

> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int da9062_thermal_get_mode(struct thermal_zone_device *z,
> +				   enum thermal_device_mode *mode)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +	*mode = thermal->mode;
> +	return 0;
> +}
> +
> +static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
> +				int trip,
> +				enum thermal_trip_type *type)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	switch (trip) {
> +	case 0:
> +		*type = THERMAL_TRIP_HOT;
> +		break;
> +	default:
> +		dev_err(thermal->dev,
> +			"Driver does not support more than 1 trip-wire\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int da9062_thermal_get_trip_temp(struct thermal_zone_device *z,
> +					int trip,
> +					int *temp)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	switch (trip) {
> +	case 0:
> +		*temp = DA9062_MILLI_CELSIUS(125);
> +		break;
> +	default:
> +		dev_err(thermal->dev,
> +			"Driver does not support more than 1 trip-wire\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int da9062_thermal_get_temp(struct thermal_zone_device *z,
> +				   int *temp)
> +{
> +	struct da9062_thermal *thermal = z->devdata;
> +
> +	mutex_lock(&thermal->lock);
> +	*temp = thermal->temperature;
> +	mutex_unlock(&thermal->lock);
> +
> +	return 0;
> +}
> +
> +static struct thermal_zone_device_ops da9062_thermal_ops = {
> +	.get_temp	= da9062_thermal_get_temp,
> +	.get_mode	= da9062_thermal_get_mode,
> +	.get_trip_type	= da9062_thermal_get_trip_type,
> +	.get_trip_temp	= da9062_thermal_get_trip_temp,
> +};
> +
> +static const struct da9062_thermal_config da9062_config = {
> +	.name = "da9062-thermal",
> +};
> +
> +static const struct of_device_id da9062_compatible_reg_id_table[] = {
> +	{ .compatible = "dlg,da9062-thermal", .data = &da9062_config },
> +	{ },
> +};
> +
> +static int da9062_thermal_probe(struct platform_device *pdev)
> +{
> +	struct da9062 *chip = dev_get_drvdata(pdev->dev.parent);
> +	struct da9062_thermal *thermal;
> +	unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> +	const struct of_device_id *match;
> +	int ret = 0;
> +
> +	match = of_match_node(da9062_compatible_reg_id_table,
> +			      pdev->dev.of_node);
> +	if (!match)
> +		return -ENXIO;
> +
> +	if (pdev->dev.of_node) {
> +		if (!of_property_read_u32(pdev->dev.of_node,
> +					"dlg,tjunc-temp-polling-period-ms",
> +					&pp_tmp)) {
> +			if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD ||
> +				pp_tmp > DA9062_MAX_POLLING_MS_PERIOD)
> +				pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD;
> +		}
> +
> +		dev_dbg(&pdev->dev,
> +			 "TJUNC temp polling period set at %d ms\n",
> +			 pp_tmp);
> +	}
> +
> +	thermal = devm_kzalloc(&pdev->dev, sizeof(struct da9062_thermal),
> +			     GFP_KERNEL);
> +	if (!thermal) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	thermal->config = match->data;
> +	thermal->hw = chip;
> +	thermal->polling_period = pp_tmp;
> +	thermal->mode = THERMAL_DEVICE_ENABLED;
> +	thermal->dev = &pdev->dev;
> +
> +	INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> +	mutex_init(&thermal->lock);
> +
> +	thermal->zone = thermal_zone_device_register(thermal->config->name,
> +					1, 0, thermal,
> +					&da9062_thermal_ops, NULL, 0,
> +					0);

Did you try using of-thermal?

So you would avoid having specific DT properties for something that
there is already a defined property?

> +	if (IS_ERR(thermal->zone)) {
> +		dev_err(&pdev->dev, "Cannot register thermal zone device\n");
> +		ret = PTR_ERR(thermal->zone);
> +		goto err;
> +	}
> +
> +	ret = platform_get_irq_byname(pdev, "THERMAL");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to get platform IRQ.\n");
> +		goto err_zone;
> +	}
> +	thermal->irq = ret;
> +
> +	ret = request_threaded_irq(thermal->irq, NULL,
> +				   da9062_thermal_irq_handler,
> +				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +				   "THERMAL", thermal);

How about using the devm_ version?

> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Failed to request thermal device IRQ.\n");
> +		goto err_zone;
> +	}
> +
> +	platform_set_drvdata(pdev, thermal);
> +	return 0;
> +
> +err_zone:
> +	thermal_zone_device_unregister(thermal->zone);
> +err:
> +	return ret;
> +}
> +
> +static int da9062_thermal_remove(struct platform_device *pdev)
> +{
> +	struct	da9062_thermal *thermal = platform_get_drvdata(pdev);
> +
> +	free_irq(thermal->irq, thermal);
> +	thermal_zone_device_unregister(thermal->zone);
> +	cancel_delayed_work_sync(&thermal->work);
> +	return 0;
> +}
> +
> +static struct platform_driver da9062_thermal_driver = {
> +	.probe	= da9062_thermal_probe,
> +	.remove	= da9062_thermal_remove,
> +	.driver	= {
> +		.name	= "da9062-thermal",
> +		.of_match_table = da9062_compatible_reg_id_table,
> +	},
> +};
> +
> +module_platform_driver(da9062_thermal_driver);
> +
> +MODULE_AUTHOR("Steve Twiss");
> +MODULE_DESCRIPTION("Thermal TJUNC device driver for Dialog DA9062 and DA9061");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:da9062-thermal");
> -- 
> end-of-patch for PATCH V2
> 
--
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

* [PATCH v4] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-29  1:21 UTC (permalink / raw)
  To: Russell King - ARM Linux, Stephen Boyd, Rob Herring, Linux-ALSA,
	Linux-DT, Michael Turquette, Linux-Kernel, Mark Brown,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM


From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
This patch adds it. It is implemeted in clk-devres.c to share
devm_clk_release().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
v3 -> v4

 - git log explain why it is implemeted in clk-devres
 - it is related to CONFIG_HAVE_CLK

 drivers/clk/clk-devres.c | 21 +++++++++++++++++++++
 include/linux/clk.h      | 27 +++++++++++++++++++++++----
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 8f57154..2449b25 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -53,3 +53,24 @@ void devm_clk_put(struct device *dev, struct clk *clk)
 	WARN_ON(ret);
 }
 EXPORT_SYMBOL(devm_clk_put);
+
+struct clk *devm_of_clk_get(struct device *dev,
+			    struct device_node *np, int index)
+{
+	struct clk **ptr, *clk;
+
+	ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	clk = of_clk_get(np, index);
+	if (!IS_ERR(clk)) {
+		*ptr = clk;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return clk;
+}
+EXPORT_SYMBOL(devm_of_clk_get);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 123c027..7f50c5f 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -17,8 +17,9 @@
 #include <linux/notifier.h>
 
 struct device;
-
 struct clk;
+struct device_node;
+struct of_phandle_args;
 
 /**
  * DOC: clk notifier callback types
@@ -249,6 +250,21 @@ static inline void clk_unprepare(struct clk *clk)
 struct clk *devm_clk_get(struct device *dev, const char *id);
 
 /**
+ * devm_clk_get - lookup and obtain a managed reference to a clock producer.
+ * @dev: device for clock "consumer"
+ * @np: pointer to clock consumer node
+ * @index: clock index
+ *
+ * This function parses the clocks, and uses them to look up the
+ * struct clk from the registered list of clock providers by using
+ * @np and @index.
+ *
+ * The clock will automatically be freed when the device is unbound
+ * from the bus.
+ */
+struct clk *devm_of_clk_get(struct device *dev, struct device_node *np, int index);
+
+/**
  * clk_enable - inform the system when the clock source should be running.
  * @clk: clock source
  *
@@ -432,6 +448,12 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
 	return NULL;
 }
 
+static inline struct clk *devm_of_clk_get(struct device *dev,
+					  struct device_node *np, int index)
+{
+	return NULL;
+}
+
 static inline void clk_put(struct clk *clk) {}
 
 static inline void devm_clk_put(struct device *dev, struct clk *clk) {}
@@ -501,9 +523,6 @@ static inline void clk_disable_unprepare(struct clk *clk)
 	clk_unprepare(clk);
 }
 
-struct device_node;
-struct of_phandle_args;
-
 #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
 struct clk *of_clk_get(struct device_node *np, int index);
 struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
-- 
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 related

* Re: [PATCH V4 10/10] PM / OPP: Don't assume platform doesn't have regulators
From: Stephen Boyd @ 2016-11-29  1:18 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm-l0cyMroinI0, Viresh Kumar,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <82cc855c7abb5d0583abe4cc9132c7e589434814.1479986492.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 11/24, Viresh Kumar wrote:
> If the regulators aren't set explicitly by the platform, the OPP core
> assumes that the platform doesn't have any regulator and uses the
> clk-only callback.
> 
> If the platform failed to register a regulator with the core, then this
> can turn out to be a dangerous assumption as the OPP core will try to
> change clk without changing regulators.
> 
> Handle that properly by making sure that the DT didn't have any entries
> for supply voltages as well.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 V4 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks
From: Stephen Boyd @ 2016-11-29  1:16 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm, Viresh Kumar, linaro-kernel, linux-pm,
	linux-kernel, Vincent Guittot, robh, d-gerlach, broonie,
	devicetree
In-Reply-To: <743654b6d2b18f94cd36b8b700c028f67cebaada.1479986492.git.viresh.kumar@linaro.org>

On 11/24, Viresh Kumar wrote:
> diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
> index 99491f4099e5..7c945d5950bf 100644
> --- a/drivers/base/power/opp/core.c
> +++ b/drivers/base/power/opp/core.c
> @@ -673,6 +673,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>  {
>  	struct opp_table *opp_table;
>  	unsigned long freq, old_freq;
> +	int (*set_opp)(struct device *dev, struct dev_pm_set_opp_data *data);

Curious why dev is an argument that isn't part of struct
dev_pm_set_opp_data here? Is there some benefit to keeping it
split out?

>  	struct dev_pm_opp *old_opp, *opp;
>  	struct regulator **regulators;
>  	struct dev_pm_set_opp_data *data;
> @@ -1488,7 +1497,7 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
>  
>  	opp_table->regulator_count = count;
>  
> -	/* Allocate block only once to pass to ->set_rate() */
> +	/* Allocate block only once to pass to ->set_opp() */

Ah here it is.

>  	ret = _allocate_set_opp_data(opp_table);
>  	if (ret)
>  		goto free_regulators;
> @@ -1563,6 +1572,109 @@ void dev_pm_opp_put_regulators(struct device *dev)
>  EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
>  
>  /**
> + * dev_pm_opp_register_set_opp_helper() - Register custom OPP set rate helper

s/custom OPP set rate/custom set OPP/ ?

> + * @dev: Device for which the helper is getting registered.
> + * @set_opp: Custom set OPP helper.
> + *
> + * This is useful to support complex platforms (like platforms with multiple
> + * regulators per device), instead of the generic OPP set rate helper.
> + *
> + * This must be called before any OPPs are initialized for the device.
> + *
> + * Locking: The internal opp_table and opp structures are RCU protected.
> + * Hence this function internally uses RCU updater strategy with mutex locks
> + * to keep the integrity of the internal data structures. Callers should ensure
> + * that this function is *NOT* called under RCU protection or in contexts where
> + * mutex cannot be locked.
> + */

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH V4 07/10] PM / OPP: Separate out _generic_opp_set_rate()
From: Stephen Boyd @ 2016-11-29  1:09 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm-l0cyMroinI0, Viresh Kumar,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1f8634c64ccec1d24119afea4b6bd2d7d1a911cf.1479986492.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 11/24, Viresh Kumar wrote:
> Later patches would add support for custom opp_set_rate callbacks. This
> patch separates out the code for generic opp_set_rate handler in order
> to prepare for that.

s/opp_set_rate/set_opp/ twice?

> 
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> ---

Besides the naming confusion.

Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

> @@ -1422,6 +1488,11 @@ int dev_pm_opp_set_regulators(struct device *dev, const char * const names[],
>  
>  	opp_table->regulator_count = count;
>  
> +	/* Allocate block only once to pass to ->set_rate() */

_generic_set_opp()? Or just set_opp when that gets introduced in
the next patch.

> +	ret = _allocate_set_opp_data(opp_table);
> +	if (ret)
> +		goto free_regulators;
> +
>  	mutex_unlock(&opp_table_lock);
>  	return 0;
>  

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 03/10] Documentation: devicetree: thermal: da9062/61 TJUNC temperature binding
From: Eduardo Valentin @ 2016-11-29  0:59 UTC (permalink / raw)
  To: Steve Twiss
  Cc: DEVICETREE, LINUX-KERNEL, LINUX-PM, Mark Rutland, Rob Herring,
	Zhang Rui, Dmitry Torokhov, Guenter Roeck, LINUX-INPUT,
	LINUX-WATCHDOG, Lee Jones, Liam Girdwood, Mark Brown,
	Support Opensource, Wim Van Sebroeck
In-Reply-To: <d8c986e39bafc2b8a1c5c23e722f61cdd021e9be.1477501000.git.stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>

On Wed, Oct 26, 2016 at 05:56:37PM +0100, Steve Twiss wrote:
> From: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> 
> Device tree binding information for DA9062 and DA9061 thermal junction
> temperature monitor.
> 
> Binding descriptions for the DA9061 and DA9062 thermal TJUNC supervisor
> device driver, using a single THERMAL_TRIP_HOT trip-wire and allowing for
> a configurable polling period for over-temperature polling.
> 
> Signed-off-by: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> 
> ---
> This patch applies against linux-next and v4.8
> 
> v1 -> v2
>  - Patch renamed from [PATCH V1 08/10] to [PATCH V2 03/10] -- these
>    changes were made to fix checkpatch warnings caused by the patch
>    set dependency order
>  - A second example for DA9061 is provided to highlight the use of a
>    fall-back compatible option for the DA9062 watchdog driver
> 
> Hi,
> 
> This patch depends on acceptance of the main code for the thermal device
> driver:
>   [PATCH V2 09/10] thermal: da9061: TJUNC temperature driver 
> 
> The previous [PATCH V1 08/10] was acked-by: Rob Herring, however this has
> not been added because changes have been made to add a new binding
> example. This describes the use of DA9061.
> This addition was made after alterations to the device driver meant that a
> fall-back compatible string could reuse the DA9062 device driver.
>  
> Regards,
> Steve Twiss, Dialog Semiconductor Ltd.
> 
> 
>  .../devicetree/bindings/thermal/da9062-thermal.txt | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> 
> diff --git a/Documentation/devicetree/bindings/thermal/da9062-thermal.txt b/Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> new file mode 100644
> index 0000000..fb207ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> @@ -0,0 +1,37 @@
> +* Dialog DA9062/61 TJUNC Thermal Module
> +
> +This module is part of the DA9061/DA9062. For more details about entire
> +DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
> +
> +Junction temperature thermal module uses an interrupt signal to identify
> +high THERMAL_TRIP_HOT temperatures for the PMIC device.
> +
> +Required properties:
> +
> +- compatible: should be one of:
> +        dlg,da9061-thermal
> +        dlg,da9062-thermal
> +
> +Optional properties:
> +
> +- dlg,tjunc-temp-polling-period-ms : Specify the polling period, measured
> +    in milliseconds, between thermal zone device update checks.


Can you please elaborate on why you need this chip manufacture specific property?

Can we use the polling property of already existing in the 
Documentation/devicetree/bindings/thermal/thermal.txt

See the polling properties.

> +
> +Example: DA9061
> +
> +	pmic0: da9062@58 {
> +		thermal {
> +			compatible = "dlg,da9062-thermal";
> +			dlg,tjunc-temp-polling-period-ms = <3000>;

Same

> +		};
> +	};
> +
> +Example: DA9061 using a fall-back compatible for the DA9062 onkey driver
> +
> +	pmic0: da9061@58 {
> +		thermal {
> +			compatible = "dlg,da9061-thermal", "dlg,da9062-thermal";
> +			dlg,tjunc-temp-polling-period-ms = <3000>;

Same 
> +		};
> +	};
> +
> -- 
> end-of-patch for PATCH V2
> 
--
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 V4 05/10] PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
From: Stephen Boyd @ 2016-11-29  0:59 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm-l0cyMroinI0, Viresh Kumar,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d71095ecb8e8dde5861e73f2b8df8955f2e2fb9c.1479986491.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 11/24, Viresh Kumar wrote:
> Pass the entire supply structure instead of all of its fields.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> ---

Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 V4 04/10] PM / OPP: Manage supply's voltage/current in a separate structure
From: Stephen Boyd @ 2016-11-29  0:58 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm, Viresh Kumar, linaro-kernel, linux-pm,
	linux-kernel, Vincent Guittot, robh, d-gerlach, broonie,
	devicetree
In-Reply-To: <6078de802f62a50e4da80022949745fc9f0bdce8.1479986491.git.viresh.kumar@linaro.org>

On 11/24, Viresh Kumar wrote:
> This is a preparatory step for multiple regulator per device support.
> Move the voltage/current variables to a new structure.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Tested-by: Dave Gerlach <d-gerlach@ti.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH V4 03/10] PM / OPP: Don't use OPP structure outside of rcu protected section
From: Stephen Boyd @ 2016-11-29  0:57 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm-l0cyMroinI0, Viresh Kumar,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <7817d2d3ccc77680c6e40101e80d201794398693.1479986491.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 11/24, Viresh Kumar wrote:
> The OPP structure must not be used out of the rcu protected section.
> Cache the values to be used in separate variables instead.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> ---

Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Should this have a fixes tag?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 V4 06/10] PM / OPP: Add infrastructure to manage multiple regulators
From: Stephen Boyd @ 2016-11-29  0:55 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, nm-l0cyMroinI0, Viresh Kumar,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9ef150da6d3344d2ad4ce884f777174a3aef5dc1.1479986491.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 11/24, Viresh Kumar wrote:
> diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
> index 37fad2eb0f47..2d5c726c920f 100644
> --- a/drivers/base/power/opp/core.c
> +++ b/drivers/base/power/opp/core.c
> @@ -235,21 +240,41 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
>  		return 0;
>  	}
>  
> -	reg = opp_table->regulator;
> -	if (IS_ERR(reg)) {
> +	count = opp_table->regulator_count;
> +
> +	if (!count) {
>  		/* Regulator may not be required for device */
>  		rcu_read_unlock();
>  		return 0;
>  	}
>  
> -	list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
> -		if (!opp->available)
> -			continue;
> +	size = count * sizeof(*regulators);
> +	regulators = kmemdup(opp_table->regulators, size, GFP_KERNEL);

Again, can't allocate with sleeping calls under RCU read lock as
it may have disabled preemption.

> +	if (!regulators) {
> +		rcu_read_unlock();
> +		return 0;
> +	}
> +
> +	uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
> +	if (!uV) {
> +		kfree(regulators);
> +		rcu_read_unlock();
> +		return 0;
> +	}
>  
> -		if (opp->supply.u_volt_min < min_uV)
> -			min_uV = opp->supply.u_volt_min;
> -		if (opp->supply.u_volt_max > max_uV)
> -			max_uV = opp->supply.u_volt_max;
> +	for (i = 0; i < count; i++) {
> +		uV[i].min = ~0;
> +		uV[i].max = 0;
> +
> +		list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
> +			if (!opp->available)
> +				continue;
> +
> +			if (opp->supplies[i].u_volt_min < uV[i].min)
> +				uV[i].min = opp->supplies[i].u_volt_min;
> +			if (opp->supplies[i].u_volt_max > uV[i].max)
> +				uV[i].max = opp->supplies[i].u_volt_max;
> +		}
>  	}
>  
>  	rcu_read_unlock();
> @@ -924,35 +960,50 @@ struct dev_pm_opp *_allocate_opp(struct device *dev,
>  				 struct opp_table **opp_table)
>  {
>  	struct dev_pm_opp *opp;
> +	int count, supply_size;
> +	struct opp_table *table;
>  
> -	/* allocate new OPP node */
> -	opp = kzalloc(sizeof(*opp), GFP_KERNEL);
> -	if (!opp)
> +	table = _add_opp_table(dev);
> +	if (!table)
>  		return NULL;
>  
> -	INIT_LIST_HEAD(&opp->node);
> +	/* Allocate space for at least one supply */
> +	count = table->regulator_count ? table->regulator_count : 1;
> +	supply_size = sizeof(*opp->supplies) * count;
>  
> -	*opp_table = _add_opp_table(dev);
> -	if (!*opp_table) {
> -		kfree(opp);
> +	/* allocate new OPP node + and supplies structures */

s/+//

> +	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
> +	if (!opp) {
> +		kfree(table);
>  		return NULL;
>  	}
>  
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 v5 8/8] iommu/rockchip: Enable Rockchip IOMMU on ARM64
From: Brian Norris @ 2016-11-29  0:42 UTC (permalink / raw)
  To: Shunqian Zheng
  Cc: joro-zLv9SwRftAIdnm+yROfE0A, heiko-4mtYJXux2i+zQB+pC5nmwQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, mark.yao-TNX95d0MmH7DzftRWevZcw,
	airlied-cv59FeDIM0c, tfiga-hpIqsD4AKlfQT0dZR+AlfA,
	xxm-TNX95d0MmH7DzftRWevZcw, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Tomasz Figa,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1466734413-7453-9-git-send-email-zhengsq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Hi,

On Fri, Jun 24, 2016 at 10:13:33AM +0800, Shunqian Zheng wrote:
> From: Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> This patch makes it possible to compile the rockchip-iommu driver on
> ARM64, so that it can be used with 64-bit SoCs equipped with this type
> of IOMMU.
> 
> Signed-off-by: Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Shunqian Zheng <zhengsq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  drivers/iommu/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index ad08603..5572621 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG
>  
>  config ROCKCHIP_IOMMU
>  	bool "Rockchip IOMMU Support"
> -	depends on ARM
> +	depends on ARM || ARM64
>  	depends on ARCH_ROCKCHIP || COMPILE_TEST
>  	select IOMMU_API
>  	select ARM_DMA_USE_IOMMU

Whatever happened with the rest of this series? Some of the IOMMU bits
made it, but the DRM fixes never did, and so this didn't get applied.
This leaves the whole DRM stack unusable on ARM64 Rockchip systems.

The patch context has changed a bit on patch 7 (and maybe 6?), so
somebody will need to refresh those on the latest upstream.

Brian
--
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 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets
From: Stephen Boyd @ 2016-11-29  0:35 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kishon, robh+dt, mark.rutland, devicetree, linux-kernel,
	srinivas.kandagatla, linux-arm-msm
In-Reply-To: <1479816163-5260-5-git-send-email-vivek.gautam@codeaurora.org>

On 11/22, Vivek Gautam wrote:
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index f1dcec1..8970d9e 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -430,6 +430,15 @@ config PHY_STIH407_USB
>  	  Enable this support to enable the picoPHY device used by USB2
>  	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
>  
> +config PHY_QCOM_QMP
> +	tristate "Qualcomm QMP PHY Driver"
> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	select GENERIC_PHY
> +	select RESET_CONTROLLER

Again, probably should be dropped as we're not providing resets
here, just using them.

> diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
> new file mode 100644
> index 0000000..f85289e
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-qmp.c
> @@ -0,0 +1,1141 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/phy/phy.h>
> +
[...]
> +
> +/* set of registers with offsets different per-PHY */
> +enum qphy_reg_layout {
> +	/* Common block control registers */
> +	QPHY_COM_SW_RESET,
> +	QPHY_COM_POWER_DOWN_CONTROL,
> +	QPHY_COM_START_CONTROL,
> +	QPHY_COM_PCS_READY_STATUS,
> +	/* PCS registers */
> +	QPHY_PLL_LOCK_CHK_DLY_TIME,
> +	QPHY_FLL_CNTRL1,
> +	QPHY_FLL_CNTRL2,
> +	QPHY_FLL_CNT_VAL_L,
> +	QPHY_FLL_CNT_VAL_H_TOL,
> +	QPHY_FLL_MAN_CODE,
> +	QPHY_PCS_READY_STATUS,
> +};
> +
> +unsigned int pciephy_regs_layout[] = {

static? const?

> +	[QPHY_COM_SW_RESET]		= 0x400,
> +	[QPHY_COM_POWER_DOWN_CONTROL]	= 0x404,
> +	[QPHY_COM_START_CONTROL]	= 0x408,
> +	[QPHY_COM_PCS_READY_STATUS]	= 0x448,
> +	[QPHY_PLL_LOCK_CHK_DLY_TIME]	= 0xa8,
> +	[QPHY_FLL_CNTRL1]		= 0xc4,
> +	[QPHY_FLL_CNTRL2]		= 0xc8,
> +	[QPHY_FLL_CNT_VAL_L]		= 0xcc,
> +	[QPHY_FLL_CNT_VAL_H_TOL]	= 0xd0,
> +	[QPHY_FLL_MAN_CODE]		= 0xd4,
> +	[QPHY_PCS_READY_STATUS]		= 0x174,
> +};
> +
> +unsigned int usb3phy_regs_layout[] = {

static? const?

> +	[QPHY_FLL_CNTRL1]		= 0xc0,
> +	[QPHY_FLL_CNTRL2]		= 0xc4,
> +	[QPHY_FLL_CNT_VAL_L]		= 0xc8,
> +	[QPHY_FLL_CNT_VAL_H_TOL]	= 0xcc,
> +	[QPHY_FLL_MAN_CODE]		= 0xd0,
> +	[QPHY_PCS_READY_STATUS]		= 0x17c,
> +};
> +
> +static struct qmp_phy_init_tbl pciephy_serdes_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x1c),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_EN, 0x42),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0xff),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x1f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORECLK_DIV, 0x0a),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x09),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x1a),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x0a),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x02),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_BUF_ENABLE, 0x1f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x04),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN1_MODE0, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x02),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0x2f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x19),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_RESCODE_DIV_NUM, 0x15),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_EP_DIV, 0x19),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_RESCODE_DIV_NUM, 0x40),
> +};
> +
> +static struct qmp_phy_init_tbl pciephy_tx_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06),
> +};
> +
> +static struct qmp_phy_init_tbl pciephy_rx_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_ENABLES, 0x1c),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xdb),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_BAND, 0x18),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN_HALF, 0x04),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x14),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x19),
> +};
> +
> +static struct qmp_phy_init_tbl pciephy_pcs_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QPHY_RX_IDLE_DTCT_CNTRL, 0x4c),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_PWRUP_RESET_DLY_TIME_AUXCLK, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_LP_WAKEUP_DLY_TIME_AUXCLK, 0x01),
> +
> +	QCOM_QMP_PHY_INIT_CFG_L(QPHY_PLL_LOCK_CHK_DLY_TIME, 0x05),
> +
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_ENDPOINT_REFCLK_DRIVE, 0x05),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x02),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG4, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG1, 0xa3),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_TXDEEMPH_M3P5DB_V0, 0x0e),
> +};
> +
> +static struct qmp_phy_init_tbl usb3phy_serdes_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x14),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x08),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x30),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x04),
> +	/* PLL and Loop filter settings */
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x15),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x34),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_CFG, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x0a),
> +	/* SSC settings */
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0xde),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x07),
> +};
> +
> +static struct qmp_phy_init_tbl usb3phy_tx_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06),
> +};
> +
> +static struct qmp_phy_init_tbl usb3phy_rx_init_tbl[] = {

const?

> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x02),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4c),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xbb),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x03),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x18),
> +	QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x16),
> +};
> +
> +static struct qmp_phy_init_tbl usb3phy_pcs_init_tbl[] = {

const?

> +	/* FLL settings */
> +	QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL2, 0x03),
> +	QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL1, 0x02),
> +	QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_L, 0x09),
> +	QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_H_TOL, 0x42),
> +	QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_MAN_CODE, 0x85),
> +
> +	/* Lock Det settings */
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG1, 0xd1),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG2, 0x1f),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG3, 0x47),
> +	QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG2, 0x08),
> +};
> +
> +/**
> + * struct qmp_phy_init_cfg:- per-PHY init config.

The colon and dash can't be right. One is kernel-doc, but of
course there aren't any other member descriptions.

> + */
> +struct qmp_phy_init_cfg {
> +	/* phy-type - PCIE/UFS/USB */
> +	unsigned int type;
> +	/* number of lanes provided by phy */
> +	int nlanes;
> +
> +	/* Initialization sequence for PHY blocks - Serdes, tx, rx, pcs */
> +	struct qmp_phy_init_tbl *phy_init_serdes_tbl;
> +	int phy_init_serdes_tbl_sz;
> +	struct qmp_phy_init_tbl *phy_init_tx_tbl;
> +	int phy_init_tx_tbl_sz;
> +	struct qmp_phy_init_tbl *phy_init_rx_tbl;
> +	int phy_init_rx_tbl_sz;
> +	struct qmp_phy_init_tbl *phy_init_pcs_tbl;
> +	int phy_init_pcs_tbl_sz;

_sz makes it sound like bytes, perhaps _num instead.

> +
> +	/* array of registers with different offsets */
> +	unsigned int *regs;

const?

> +
> +	unsigned int mask_start_ctrl;
> +	unsigned int mask_pwr_dn_ctrl;
> +	/* true, if PHY has a separate PHY_COM_CNTRL block */
> +	bool has_phy_com_ctrl;
> +	/* true, if PHY has a reset for individual lanes */
> +	bool has_lane_rst;
> +};
> +
> +/**
> + * struct qmp_phy_desc:- per-lane phy-descriptor.

Also kernel-doc requests full stop is left out on one line
descriptions.

> + *
> + * @phy: pointer to generic phy
> + * @tx: pointer to iomapped memory space for PHY's tx
> + * @rx: pointer to iomapped memory space for PHY's rx
> + * @pcs: pointer to iomapped memory space for PHY's pcs
> + * @pipe_clk: pointer to pipe lock
> + * @index: lane index
> + * @qphy: pointer to QMP phy to which this lane belongs
> + * @lane_rst: pointer to lane's reset controller

Not sure we care about "pointer to" as types can be figured out
other ways.

> + */
> +struct qmp_phy_desc {
> +	struct phy *phy;
> +	void __iomem *tx;
> +	void __iomem *rx;
> +	void __iomem *pcs;
> +	struct clk *pipe_clk;
> +	unsigned int index;
> +	struct qcom_qmp_phy *qphy;
> +	struct reset_control *lane_rst;
> +};
> +
> +/**
> + * struct qcom_qmp_phy:- structure holding QMP PHY attributes.
> + *
> + * @dev: pointer to device
> + * @serdes: pointer to iomapped memory space for phy's serdes
> + *
> + * @aux_clk: pointer to phy core clock
> + * @cfg_ahb_clk: pointer to AHB2PHY interface clock
> + * @ref_clk: pointer to reference clock
> + * @ref_clk_src: pointer to source to reference clock
> + *
> + * @vdda_phy: vdd supply to the phy core block
> + * @vdda_pll: 1.8V vdd supply to ref_clk block
> + * @vddp_ref_clk: vdd supply to specific ref_clk block (Optional)
> + *
> + * @phy_rst: Pointer to phy reset control
> + * @phycom_rst: Pointer to phy common reset control
> + * @phycfg_rst: Pointer to phy ahb cfg reset control (Optional)
> + *
> + * @cfg: pointer to init config for each phys
> + * @phys: array of pointer to per-lane phy descriptors
> + * @phy_mutex: mutex lock for PHY common block initialization
> + * @init_count: Phy common block initialization count
> + */
> +struct qcom_qmp_phy {
> +	struct device *dev;
> +	void __iomem *serdes;
> +
> +	struct clk *aux_clk;
> +	struct clk *cfg_ahb_clk;
> +	struct clk *ref_clk;
> +	struct clk *ref_clk_src;
> +
> +	struct regulator *vdda_phy;
> +	struct regulator *vdda_pll;
> +	struct regulator *vddp_ref_clk;
> +
> +	struct reset_control *phy_rst;
> +	struct reset_control *phycom_rst;
> +	struct reset_control *phycfg_rst;
> +
> +	const struct qmp_phy_init_cfg *cfg;
> +	struct qmp_phy_desc **phys;
> +
> +	struct mutex phy_mutex;
> +	int init_count;
> +};
> +
> +static inline void qphy_setbits(void __iomem *reg, u32 val)
> +{
> +	u32 reg_val;
> +
> +	reg_val = readl_relaxed(reg);
> +	reg_val |= val;
> +	writel_relaxed(reg_val, reg);
> +}
> +
> +static inline void qphy_clrbits(void __iomem *reg, u32 val)
> +{
> +	u32 reg_val;
> +
> +	reg_val = readl_relaxed(reg);
> +	reg_val &= ~val;
> +	writel_relaxed(reg_val, reg);
> +}
> +
> +const struct qmp_phy_init_cfg msm8996_pciephy_init_cfg = {

static?

> +	.type			= PHY_TYPE_PCIE,
> +	.nlanes			= 3,
> +
> +	.phy_init_serdes_tbl	= pciephy_serdes_init_tbl,
> +	.phy_init_serdes_tbl_sz	= ARRAY_SIZE(pciephy_serdes_init_tbl),
> +	.phy_init_tx_tbl	= pciephy_tx_init_tbl,
> +	.phy_init_tx_tbl_sz	= ARRAY_SIZE(pciephy_tx_init_tbl),
> +	.phy_init_rx_tbl	= pciephy_rx_init_tbl,
> +	.phy_init_rx_tbl_sz	= ARRAY_SIZE(pciephy_rx_init_tbl),
> +	.phy_init_pcs_tbl	= pciephy_pcs_init_tbl,
> +	.phy_init_pcs_tbl_sz	= ARRAY_SIZE(pciephy_pcs_init_tbl),
> +	.regs			= pciephy_regs_layout,
> +	.mask_start_ctrl	= (PHY_PCS_START | PHY_PLL_READY_GATE_EN),
> +	.mask_pwr_dn_ctrl	= (PHY_SW_PWRDN | PHY_REFCLK_DRV_DSBL),
> +
> +	.has_phy_com_ctrl	= true,
> +	.has_lane_rst		= true,
> +};
> +
> +const struct qmp_phy_init_cfg msm8996_usb3phy_init_cfg = {

static? Try running sparse.

> +	.type			= PHY_TYPE_USB3,
> +	.nlanes			= 1,
> +
> +	.phy_init_serdes_tbl	= usb3phy_serdes_init_tbl,
> +	.phy_init_serdes_tbl_sz	= ARRAY_SIZE(usb3phy_serdes_init_tbl),
> +	.phy_init_tx_tbl	= usb3phy_tx_init_tbl,
> +	.phy_init_tx_tbl_sz	= ARRAY_SIZE(usb3phy_tx_init_tbl),
> +	.phy_init_rx_tbl	= usb3phy_rx_init_tbl,
> +	.phy_init_rx_tbl_sz	= ARRAY_SIZE(usb3phy_rx_init_tbl),
> +	.phy_init_pcs_tbl	= usb3phy_pcs_init_tbl,
> +	.phy_init_pcs_tbl_sz	= ARRAY_SIZE(usb3phy_pcs_init_tbl),

Do we really need phy_init as a prefix. Could just be serdes_tbl,
tx_tbl, etc?

> +	.regs			= usb3phy_regs_layout,
> +	.mask_start_ctrl	= (PHY_SERDES_START | PHY_PCS_START),
> +	.mask_pwr_dn_ctrl	= PHY_SW_PWRDN,
> +};
> +
> +static void qcom_qmp_phy_configure(void __iomem *base,
> +				unsigned int *regs_layout,
> +				struct qmp_phy_init_tbl init_tbl[],
> +				int init_tbl_sz)

Shorter variable names would make this easier to read.

> +{
> +	int i;
> +
> +	for (i = 0; i < init_tbl_sz; i++) {
> +		if (init_tbl[i].in_layout)
> +			writel_relaxed(init_tbl[i].cfg_val,
> +				base + regs_layout[init_tbl[i].reg_offset]);
> +		else
> +			writel_relaxed(init_tbl[i].cfg_val,
> +				base + init_tbl[i].reg_offset);
> +	}
> +
	const struct qmp_phy_init_tbl *t = tbl;

	for (i = 0; i < num; i++, t++)
   		if (t->in_layout)
   			writel_relaxed(t->val, base + regs[t->offset]);
   		else
   			writel_relaxed(t->val, base + t->offset);

> +	/* flush buffered writes */
> +	mb();
> +}
> +
> +static int qcom_qmp_phy_poweron(struct phy *phy)
> +{
> +	struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
> +	struct qcom_qmp_phy *qphy = phydesc->qphy;
> +	int ret;
> +
> +	dev_vdbg(&phy->dev, "Powering on QMP phy\n");
> +
> +	ret = regulator_enable(qphy->vdda_phy);
> +	if (ret) {
> +		dev_err(qphy->dev, "%s: vdda-phy enable failed, err=%d\n",
> +				__func__, ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(qphy->vdda_pll);
> +	if (ret) {
> +		dev_err(qphy->dev, "%s: vdda-pll enable failed, err=%d\n",
> +				__func__, ret);
> +		goto err_vdda_pll;
> +	}
> +
> +	if (qphy->vddp_ref_clk) {
> +		ret = regulator_enable(qphy->vddp_ref_clk);
> +		if (ret) {
> +			dev_err(qphy->dev, "%s: vdda-ref-clk enable failed, err=%d\n",
> +					__func__, ret);
> +			goto err_vddp_refclk;
> +		}
> +	}
> +
> +	clk_prepare_enable(qphy->ref_clk_src);
> +	clk_prepare_enable(qphy->ref_clk);
> +	clk_prepare_enable(phydesc->pipe_clk);

And if these fail?

> +
> +	return 0;
> +
> +err_vddp_refclk:
> +	regulator_disable(qphy->vdda_pll);
> +err_vdda_pll:
> +	regulator_disable(qphy->vdda_phy);
> +	return ret;
> +}
> +
> +static int qcom_qmp_phy_poweroff(struct phy *phy)
> +{
> +	struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
> +	struct qcom_qmp_phy *qphy = phydesc->qphy;
> +
> +	clk_disable_unprepare(qphy->ref_clk_src);
> +	clk_disable_unprepare(qphy->ref_clk);
> +	clk_disable_unprepare(phydesc->pipe_clk);
> +
> +	if (qphy->vddp_ref_clk)
> +		regulator_disable(qphy->vddp_ref_clk);
> +
> +	regulator_disable(qphy->vdda_pll);
> +	regulator_disable(qphy->vdda_phy);
> +
> +	return 0;
> +}
> +
> +static int qcom_qmp_phy_is_ready(struct qcom_qmp_phy *qphy,
> +				void __iomem *pcs_status, u32 mask)
> +{
> +	unsigned int init_timeout;
> +
> +	init_timeout = PHY_READY_TIMEOUT_COUNT;
> +	do {
> +		if (readl_relaxed(pcs_status) & mask)
> +			break;
> +
> +		usleep_range(REFCLK_STABILIZATION_DELAY_US_MIN,
> +				 REFCLK_STABILIZATION_DELAY_US_MAX);
> +	} while (--init_timeout);

readl_poll_timeout?

> +
> +	if (!init_timeout)
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +
[...]
> +
> +/* PHY Initialization */
> +static int qcom_qmp_phy_init(struct phy *phy)
> +{
> +	struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
> +	struct qcom_qmp_phy *qphy = phydesc->qphy;
> +	const struct qmp_phy_init_cfg *cfg = qphy->cfg;
> +	void __iomem *tx = phydesc->tx;
> +	void __iomem *rx = phydesc->rx;
> +	void __iomem *pcs = phydesc->pcs;
> +	int ret;
> +
> +	dev_vdbg(qphy->dev, "Initializing QMP phy\n");
> +
> +	/* enable interface clocks to program phy */
> +	clk_prepare_enable(qphy->aux_clk);
> +	clk_prepare_enable(qphy->cfg_ahb_clk);
> +
> +	ret = qcom_qmp_phy_com_init(qphy);
> +	if (ret)
> +		goto err;
> +
> +	if (phydesc->lane_rst) {
> +		ret = reset_control_deassert(phydesc->lane_rst);
> +		if (ret) {
> +			dev_err(qphy->dev, "lane<%d> reset deassert failed\n",

Drop the brackets?

> +					phydesc->index);
> +			goto err_lane_rst;
> +		}
> +	}
> +
> +	/* Tx, Rx, and PCS configurations */
> +	qcom_qmp_phy_configure(tx, cfg->regs, cfg->phy_init_tx_tbl,
> +				cfg->phy_init_tx_tbl_sz);
> +	qcom_qmp_phy_configure(rx, cfg->regs, cfg->phy_init_rx_tbl,
> +				cfg->phy_init_rx_tbl_sz);
> +	qcom_qmp_phy_configure(pcs, cfg->regs, cfg->phy_init_pcs_tbl,
> +				cfg->phy_init_pcs_tbl_sz);
> +
> +	/*
> +	 * Pull out PHY from POWER DOWN state:
> +	 * This is active low enable signal to power-down PHY.
> +	 */
> +	qphy_setbits(pcs + QPHY_POWER_DOWN_CONTROL, cfg->mask_pwr_dn_ctrl);
> +	/* XXX: 10 us delay; given in PCIE phy programming guide only */

Is this a todo?

> +	usleep_range(POWER_DOWN_DELAY_US_MIN, POWER_DOWN_DELAY_US_MAX);
> +
> +	/* start SerDes and Phy-Coding-Sublayer */
> +	qphy_setbits(pcs + QPHY_START_CTRL, cfg->mask_start_ctrl);
> +
> +	/* Pull PHY out of reset state */
> +	qphy_clrbits(pcs + QPHY_SW_RESET, PHY_SW_RESET);
> +	/* Make sure that above writes are completed */
> +	mb();
> +
> +	ret = qcom_qmp_phy_is_ready(qphy, pcs +
> +					cfg->regs[QPHY_PCS_READY_STATUS],
> +					MASK_PHYSTATUS);
> +	if (ret) {
> +		dev_err(qphy->dev, "phy initialization timed-out\n");
> +		goto err_pcs_ready;
> +	}
> +
> +	return 0;
> +
> +err_pcs_ready:
> +	if (phydesc->lane_rst)
> +		reset_control_assert(phydesc->lane_rst);
> +err_lane_rst:
> +	qcom_qmp_phy_com_exit(qphy);
> +err:
> +	clk_disable_unprepare(qphy->cfg_ahb_clk);
> +	clk_disable_unprepare(qphy->aux_clk);
> +	return ret;
> +}
> +
> +static int qcom_qmp_phy_exit(struct phy *phy)
> +{
> +	struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
> +	struct qcom_qmp_phy *qphy = phydesc->qphy;
> +	const struct qmp_phy_init_cfg *cfg = qphy->cfg;
> +
> +	/* PHY reset */
> +	qphy_setbits(phydesc->pcs + QPHY_SW_RESET, PHY_SW_RESET);
> +
> +	/* stop SerDes and Phy-Coding-Sublayer */
> +	qphy_clrbits(phydesc->pcs + QPHY_START_CTRL, cfg->mask_start_ctrl);
> +
> +	/* Put PHY into POWER DOWN state: active low */
> +	qphy_clrbits(phydesc->pcs + QPHY_POWER_DOWN_CONTROL,
> +			cfg->mask_pwr_dn_ctrl);
> +
> +	/* Make sure that above writes are completed */
> +	mb();
> +
> +	if (phydesc->lane_rst)
> +		reset_control_assert(phydesc->lane_rst);
> +
> +	qcom_qmp_phy_com_exit(qphy);
> +
> +	clk_disable_unprepare(qphy->aux_clk);
> +	clk_disable_unprepare(qphy->cfg_ahb_clk);
> +
> +	return 0;
> +}
> +
> +
> +static int qcom_qmp_phy_regulator_init(struct device *dev)
> +{
> +	struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
> +	int ret;
> +
> +	qphy->vdda_phy = devm_regulator_get(dev, "vdda-phy");
> +	if (IS_ERR(qphy->vdda_phy)) {
> +		ret = PTR_ERR(qphy->vdda_phy);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get vdda-phy, %d\n", ret);
> +		return ret;
> +	}
> +
> +	qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll");
> +	if (IS_ERR(qphy->vdda_pll)) {
> +		ret = PTR_ERR(qphy->vdda_pll);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get vdda-pll, %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* optional regulator */
> +	qphy->vddp_ref_clk = devm_regulator_get(dev, "vddp-ref-clk");
> +	if (IS_ERR(qphy->vddp_ref_clk)) {
> +		ret = PTR_ERR(qphy->vddp_ref_clk);
> +		if (ret != -EPROBE_DEFER) {
> +			dev_dbg(dev, "failed to get vddp-ref-clk, %d\n", ret);
> +			qphy->vddp_ref_clk = NULL;
> +		} else {
> +			return ret;
> +		}
> +	}

Regulator framework should insert a dummy regulator if this is
missing in DT. So we shouldn't need to do anything complicated
here right?

> +
> +	return 0;
> +}
> +
> +static int qcom_qmp_phy_clk_init(struct device *dev)
> +{
> +	struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
> +	int ret;
> +
> +	qphy->aux_clk = devm_clk_get(dev, "aux");
> +	if (IS_ERR(qphy->aux_clk)) {
> +		ret = PTR_ERR(qphy->aux_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get aux_clk\n");
> +		return ret;
> +	}
> +
> +	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
> +	if (IS_ERR(qphy->cfg_ahb_clk)) {
> +		ret = PTR_ERR(qphy->cfg_ahb_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get cfg_ahb_clk\n");
> +		return ret;
> +	}
> +
> +	qphy->ref_clk_src = devm_clk_get(dev, "ref_clk_src");
> +	if (IS_ERR(qphy->ref_clk_src)) {
> +		ret = PTR_ERR(qphy->ref_clk_src);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get ref_clk_src\n");
> +		return ret;
> +	}
> +
> +	qphy->ref_clk = devm_clk_get(dev, "ref_clk");
> +	if (IS_ERR(qphy->ref_clk)) {
> +		ret = PTR_ERR(qphy->ref_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get ref_clk\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct phy *qcom_qmp_phy_xlate(struct device *dev,
> +					struct of_phandle_args *args)
> +{
> +	struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
> +	int i;
> +
> +	if (WARN_ON(args->args[0] >= qphy->cfg->nlanes))
> +		return ERR_PTR(-ENODEV);
> +
> +	for (i = 0; i < qphy->cfg->nlanes; i++) {
> +		if (qphy->phys[i]->index == args->args[0])
> +			break;
> +	}

Drop the braces please.

> +
> +	if (i == qphy->cfg->nlanes)
> +		return ERR_PTR(-ENODEV);
> +
> +	return qphy->phys[i]->phy;

Could be simplified:


	for (i = 0; i < qphy->cfg->nlanes; i++)
		if (qphy->phys[i]->index == args->args[0])
			return qphy->phys[i]->phy;

	return ERR_PTR(-ENODEV);


Also, isn't i == phys[i]->index so this could be a direct lookup?

> +}
> +
> +static const struct phy_ops qcom_qmp_phy_gen_ops = {
> +	.init		= qcom_qmp_phy_init,
> +	.exit		= qcom_qmp_phy_exit,
> +	.power_on	= qcom_qmp_phy_poweron,
> +	.power_off	= qcom_qmp_phy_poweroff,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
> +	{
> +		.compatible = "qcom,msm8996-qmp-pcie-phy",
> +		.data = &msm8996_pciephy_init_cfg,
> +	}, {
> +		.compatible = "qcom,msm8996-qmp-usb3-phy",
> +		.data = &msm8996_usb3phy_init_cfg,
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_qmp_phy_of_match_table);
> +
> +static int qcom_qmp_phy_probe(struct platform_device *pdev)
> +{
> +	struct qcom_qmp_phy *qphy;
> +	struct device *dev = &pdev->dev;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct of_device_id *match;
> +	void __iomem *base;
> +	int ret = 0;

Shouldn't need a default assignment here.

> +	int id;
> +
> +	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
> +	if (!qphy)
> +		return -ENOMEM;
> +
> +	qphy->dev = dev;
> +	dev_set_drvdata(dev, qphy);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	/* per PHY serdes; usually located at base address */
> +	qphy->serdes = base;
> +
> +	mutex_init(&qphy->phy_mutex);
> +
> +	/* Get the specific init parameters of QMP phy */
> +	match = of_match_node(qcom_qmp_phy_of_match_table, dev->of_node);
> +	qphy->cfg = match->data;
> +
> +	ret = qcom_qmp_phy_clk_init(dev);
> +	if (ret) {
> +		dev_err(dev, "clock init failed\n");
> +		return ret;
> +	}
> +
> +	ret = qcom_qmp_phy_regulator_init(dev);
> +	if (ret) {
> +		dev_err(dev, "regulator init failed\n");
> +		return ret;
> +	}
> +
> +	qphy->phy_rst = devm_reset_control_get(dev, "phy");
> +	if (IS_ERR(qphy->phy_rst)) {
> +		dev_err(dev, "failed to get phy core reset\n");
> +		return PTR_ERR(qphy->phy_rst);
> +	}
> +
> +	qphy->phycom_rst = devm_reset_control_get(dev, "common");
> +	if (IS_ERR(qphy->phycom_rst)) {
> +		dev_err(dev, "failed to get phy common reset\n");
> +		return PTR_ERR(qphy->phycom_rst);
> +	}
> +
> +	qphy->phycfg_rst = devm_reset_control_get(dev, "cfg");
> +	if (IS_ERR(qphy->phycfg_rst)) {
> +		dev_dbg(dev, "failed to get phy ahb cfg reset\n");
> +		qphy->phycfg_rst = NULL;
> +	}
> +
> +	qphy->phys = devm_kcalloc(dev, qphy->cfg->nlanes,
> +					sizeof(*qphy->phys), GFP_KERNEL);
> +	if (!qphy->phys)
> +		return -ENOMEM;
> +
> +	for (id = 0; id < qphy->cfg->nlanes; id++) {
> +		struct phy *generic_phy;
> +		struct qmp_phy_desc *phy_desc;
> +		char prop_name[MAX_PROP_NAME];
> +		unsigned int lane_offsets[3];
> +
> +		/* mem resources from index 1 to N for N number of lanes */
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
> +		base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(base))
> +			return PTR_ERR(base);
> +
> +		phy_desc = devm_kzalloc(dev, sizeof(*phy_desc), GFP_KERNEL);
> +		if (!phy_desc)
> +			return -ENOMEM;
> +
> +		/*
> +		 * read offsets to Tx, Rx, and PCS blocks into a u32 array:
> +		 *  ------------------------------------
> +		 * | tx offset | rx offset | pcs offset |
> +		 *  ------------------------------------
> +		 */
> +		ret = of_property_read_u32_array(dev->of_node, "lane-offsets",
> +							   lane_offsets, 3);
> +		if (ret) {
> +			dev_err(dev,
> +				"failed to get tx/rx/pcs offsets for lane%d\n",
> +				id);
> +				return ret;
> +		}
> +
> +		phy_desc->tx = base + lane_offsets[0];
> +		phy_desc->rx = base + lane_offsets[1];
> +		phy_desc->pcs = base + lane_offsets[2];
> +
> +		/*
> +		 * Get PHY's Pipe clock, if any; USB3 and PCIe are PIPE3
> +		 * based phys, so they essentially have pipe clock. So,
> +		 * we return error in case phy is USB3 or PIPE type.
> +		 * Otherwise, we initialize pipe clock to NULL for
> +		 * all phys that don't need this.
> +		 */
> +		memset(&prop_name, 0, sizeof(prop_name));
> +		snprintf(prop_name, MAX_PROP_NAME, "pipe%d", id);
> +		phy_desc->pipe_clk = devm_clk_get(dev, prop_name);
> +		if (IS_ERR(phy_desc->pipe_clk)) {
> +			if (qphy->cfg->type == PHY_TYPE_PCIE ||
> +			    qphy->cfg->type == PHY_TYPE_USB3) {
> +				ret = PTR_ERR(phy_desc->pipe_clk);
> +				if (ret != -EPROBE_DEFER)
> +					dev_err(dev,
> +					"failed to get lane%d pipe_clk\n", id);
> +				return ret;
> +			} else {
> +				phy_desc->pipe_clk = NULL;
> +			}

Drop the else part.

> +		}
> +
> +		/* Get lane reset, if any */
> +		if (qphy->cfg->has_lane_rst) {
> +			memset(&prop_name, 0, sizeof(prop_name));
> +			snprintf(prop_name, MAX_PROP_NAME, "lane%d", id);
> +			phy_desc->lane_rst = devm_reset_control_get(dev,
> +								    prop_name);
> +			if (IS_ERR(phy_desc->lane_rst)) {
> +				dev_err(dev, "failed to get lane%d reset\n",
> +					id);
> +				return PTR_ERR(phy_desc->lane_rst);
> +			}
> +		}
> +
> +		generic_phy = devm_phy_create(dev, NULL, &qcom_qmp_phy_gen_ops);
> +		if (IS_ERR(generic_phy)) {
> +			ret = PTR_ERR(generic_phy);
> +			dev_err(dev, "failed to create qphy %d\n", ret);
> +			return ret;
> +		}
> +
> +		phy_desc->phy = generic_phy;
> +		phy_desc->index = id;
> +		phy_desc->qphy = qphy;
> +		phy_set_drvdata(generic_phy, phy_desc);
> +		qphy->phys[id] = phy_desc;

Probably should make the above part of this loop a function that
returns a phy for each lane (or an  error code on failure). This
probe function is long.

> +	}
> +
> +	phy_provider = devm_of_phy_provider_register(dev, qcom_qmp_phy_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		ret = PTR_ERR(phy_provider);
> +		dev_err(dev, "failed to register qphy %d\n", ret);
> +	}
> +
> +	return ret;
> +}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH] arm64: dts: Add symlinks for cros-ec-keyboard and cros-ec-sbs
From: Brian Norris @ 2016-11-28 23:51 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: olof-nZhT3qVonbNeoWH0uzbU5w, Arnd Bergmann, Heiko Stuebner,
	robh-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1463090530-21864-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Hi Olof, Arnd,

On Thu, May 12, 2016 at 03:02:10PM -0700, Doug Anderson wrote:
> We'd like to be able to use the cros-ec-keyboard.dtsi and
> cros-ec-sbs.dtsi snippets for arm64 devices.  Currently those files live
> in the arm/boot/dts directory.
> 
> Let's follow the convention set by commit 8ee57b8182c4 ("ARM64: dts:
> vexpress: Use a symlink to vexpress-v2m-rs1.dtsi from arch=arm") and use
> a symlink.  Note that in this case we put the files in a new
> "include/common" directory since these snippets may need to be
> referenced by dts files in many different subdirectories.
> 
> Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> Note that, as of right now, there are no users of this.  However, given
> development happening it is almost 100% certain that users will arrive
> soon.  If we need to wait for the first user before landing this we can
> leave this on the back burner.  If it's OK to have no users (yet), let's
> land.

This may not fit your definition of "soon", but I'm looking to start
using these files in arch/arm64/. I'll probably carry along this patch
and resend when I'm ready, but it'd be just as well if you'd merge it
now (or express a preference for a different directory structure).

>  arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi | 1 +
>  arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi      | 1 +
>  2 files changed, 2 insertions(+)
>  create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
>  create mode 120000 arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> 
> \ No newline at end of file
> 
> diff --git a/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> new file mode 120000
> index 000000000000..1c1889f0a791
> --- /dev/null
> +++ b/arch/arm64/boot/dts/include/common/cros-ec-keyboard.dtsi
> @@ -0,0 +1 @@
> +../../../../../arm/boot/dts/cros-ec-keyboard.dtsi
> \ No newline at end of file
> diff --git a/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> new file mode 120000
> index 000000000000..3d7ae9c88bcd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/include/common/cros-ec-sbs.dtsi
> @@ -0,0 +1 @@
> +../../../../../arm/boot/dts/cros-ec-sbs.dtsi

FWIW:

Reviewed-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@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

* Re: [PATCH v2 3/4] dt-bindings: phy: Add support for QMP phy
From: Stephen Boyd @ 2016-11-28 23:19 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kishon, robh+dt, mark.rutland, devicetree, linux-kernel,
	srinivas.kandagatla, linux-arm-msm
In-Reply-To: <1479816163-5260-4-git-send-email-vivek.gautam@codeaurora.org>

On 11/22, Vivek Gautam wrote:
> Qualcomm chipsets have QMP phy controller that provides
> support to a number of controller, viz. PCIe, UFS, and USB.
> Adding dt binding information for the same.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> 
> Changes since v1:
>  - New patch, forked out of the original driver patch:
>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>  - updated bindings to include mem resource as a list of
>    offset - length pair for serdes block and for each lane.
>  - added a new binding for 'lane-offsets' that contains offsets
>    to tx, rx and pcs blocks from each lane base address.
> 
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 74 ++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> new file mode 100644
> index 0000000..ffb173b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -0,0 +1,74 @@
> +Qualcomm QMP PHY
> +----------------
> +
> +QMP phy controller supports physical layer functionality for a number of
> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
> +
> +Required properties:
> + - compatible: compatible list, contains:
> +	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
> +	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
> + - reg: list of offset and length pair of the PHY register sets.
> +	at index 0: offset and length of register set for PHY common
> +		    serdes block.
> +	from index 1 - N: offset and length of register set for each lane,
> +			  for N number of phy lanes (ports).
> + - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
> + - #phy-cells: must be 1
> +    - Cell after phy phandle should be the port (lane) number.
> + - clocks: a list of phandles and clock-specifier pairs,
> +	   one for each entry in clock-names.
> + - clock-names: must be "cfg_ahb" for phy config clock,
> +			"aux" for phy aux clock,
> +			"ref_clk" for 19.2 MHz ref clk,
> +			"ref_clk_src" for reference clock source,
> +			"pipe<port-number>" for pipe clock specific to
> +			each port/lane (Optional).
> + - resets: a list of phandles and reset controller specifier pairs,
> +	   one for each entry in reset-names.
> + - reset-names: must be "phy" for reset of phy block,
> +			"common" for phy common block reset,
> +			"cfg" for phy's ahb cfg block reset (Optional).
> +			"port<port-number>" for reset specific to
> +			each port/lane (Optional).
> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +Optional properties:
> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
> +			pll block.
> +
> +Example:
> +	pcie_phy: pciephy@34000 {
> +		compatible = "qcom,msm8996-qmp-pcie-phy";
> +		reg = <0x034000 0x48f>,
> +			<0x035000 0x5bf>,
> +			<0x036000 0x5bf>,
> +			<0x037000 0x5bf>;
> +				/* tx, rx, pcs */
> +		lane-offsets = <0x0 0x200 0x400>;
> +		#phy-cells = <1>;
> +
> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
> +			<&rpmcc MSM8996_RPM_SMD_LN_BB_CLK>,
> +			<&gcc GCC_PCIE_CLKREF_CLK>,
> +			<&gcc GCC_PCIE_0_PIPE_CLK>,
> +			<&gcc GCC_PCIE_1_PIPE_CLK>,
> +			<&gcc GCC_PCIE_2_PIPE_CLK>;
> +		clock-names = "aux", "cfg_ahb",
> +				"ref_clk_src", "ref_clk",

Does MSM8996_RPM_SMD_LN_BB_CLK supply the clock source for
GCC_PCIE_CLKREF_CLK? Did we mess up the parent/child relationship
in the GCC driver? We may want to fix that so that this node
only references clocks that actually go into the device, instead
of clock parents.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v2 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
From: Stephen Boyd @ 2016-11-28 23:14 UTC (permalink / raw)
  To: Vivek Gautam
  Cc: kishon-l0cyMroinI0, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479816163-5260-3-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On 11/22, Vivek Gautam wrote:
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..f1dcec1 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -430,6 +430,17 @@ config PHY_STIH407_USB
>  	  Enable this support to enable the picoPHY device used by USB2
>  	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
>  
> +config PHY_QCOM_QUSB2
> +	tristate "Qualcomm QUSB2 PHY Driver"
> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	select GENERIC_PHY
> +	select RESET_CONTROLLER

This shouldn't be necessary. We only need to select it if we're
providing resets.

> +	help
> +	  Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
> +	  controllers on Qualcomm chips. This driver supports the high-speed
> +	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
> +	  USB IPs on MSM SOCs.
> +
>  config PHY_QCOM_UFS
>  	tristate "Qualcomm UFS PHY driver"
>  	depends on OF && ARCH_QCOM
> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
> new file mode 100644
> index 0000000..d3f9657
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-qusb2.c
> @@ -0,0 +1,549 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define QUSB2PHY_PLL_TEST		0x04
> +#define CLK_REF_SEL			BIT(7)
> +
> +#define QUSB2PHY_PLL_TUNE		0x08
> +#define QUSB2PHY_PLL_USER_CTL1		0x0c
> +#define QUSB2PHY_PLL_USER_CTL2		0x10
> +#define QUSB2PHY_PLL_AUTOPGM_CTL1	0x1c
> +#define QUSB2PHY_PLL_PWR_CTRL		0x18
> +
> +#define QUSB2PHY_PLL_STATUS		0x38
> +#define PLL_LOCKED			BIT(5)
> +
> +#define QUSB2PHY_PORT_TUNE1             0x80
> +#define QUSB2PHY_PORT_TUNE2             0x84
> +#define QUSB2PHY_PORT_TUNE3             0x88
> +#define QUSB2PHY_PORT_TUNE4             0x8C
> +#define QUSB2PHY_PORT_TUNE5		0x90
> +#define QUSB2PHY_PORT_TEST2		0x9c

Please use lowercase or uppercase consistently (I'd prefer
lowercase).

> +
> +#define QUSB2PHY_PORT_POWERDOWN		0xB4
> +#define CLAMP_N_EN			BIT(5)
> +#define FREEZIO_N			BIT(1)
> +#define POWER_DOWN			BIT(0)
> +
> +#define QUSB2PHY_REFCLK_ENABLE		BIT(0)
> +
> +#define PHY_CLK_SCHEME_SEL		BIT(0)
> +
> +struct qusb2_phy_init_tbl {
> +	unsigned int reg_offset;
> +	unsigned int cfg_val;
> +};
> +#define QCOM_QUSB2_PHY_INIT_CFG(reg, val) \
> +	{				\
> +		.reg_offset = reg,	\
> +		.cfg_val = val,		\
> +	}
> +
> +static struct qusb2_phy_init_tbl msm8996_phy_init_tbl[] = {

const?

> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xF8),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xB3),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xC0),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9F),
> +	QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> +};
> +
> +struct qusb2_phy_init_cfg {
> +	struct qusb2_phy_init_tbl *phy_init_tbl;
> +	int phy_init_tbl_sz;
> +	/* offset to PHY_CLK_SCHEME register in TCSR map. */
> +	unsigned int clk_scheme_offset;
> +};
> +
> +const struct qusb2_phy_init_cfg msm8996_phy_init_cfg = {

static?

> +	.phy_init_tbl = msm8996_phy_init_tbl,
> +	.phy_init_tbl_sz = ARRAY_SIZE(msm8996_phy_init_tbl),
> +};
> +
> +/**
> + * struct qusb2_phy: Structure holding qusb2 phy attributes.
> + *
> + * @phy: pointer to generic phy.
> + * @base: pointer to iomapped memory space for qubs2 phy.
> + *
> + * @cfg_ahb_clk: pointer to AHB2PHY interface clock.
> + * @ref_clk: pointer to reference clock.
> + * @ref_clk_src: pointer to source to reference clock.
> + * @iface_src: pointer to phy interface clock.
> + *
> + * @phy_reset: Pointer to phy reset control
> + *
> + * @vdda_phy: vdd supply to the phy core block.
> + * @vdda_pll: 1.8V vdd supply to ref_clk block.
> + * @vdda_phy_dpdm: 3.1V vdd supply to Dp/Dm port signals.
> + * @tcsr: pointer to TCSR syscon register map.

Drop all the full stops on these comments please.

> + *
> + * @cfg: phy initialization config data
> + * @has_se_clk_scheme: indicate if PHY has Single-ended ref clock scheme

Why is single capitalized?

> + */
> +struct qusb2_phy {
> +	struct phy *phy;
> +	void __iomem *base;
> +
> +	struct clk *cfg_ahb_clk;
> +	struct clk *ref_clk;
> +	struct clk *ref_clk_src;
> +	struct clk *iface_clk;
> +
> +	struct reset_control *phy_reset;
> +
> +	struct regulator *vdd_phy;
> +	struct regulator *vdda_pll;
> +	struct regulator *vdda_phy_dpdm;
> +
> +	struct regmap *tcsr;
> +
> +	const struct qusb2_phy_init_cfg *cfg;
> +	bool has_se_clk_scheme;
> +};
> +
> +static inline void qusb2_setbits(void __iomem *reg, u32 val)
> +{
> +	u32 reg_val;
> +
> +	reg_val = readl_relaxed(reg);
> +	reg_val |= val;
> +	writel_relaxed(reg_val, reg);
> +
> +	/* Ensure above write is completed */
> +	mb();
> +}
> +
> +static inline void qusb2_clrbits(void __iomem *reg, u32 val)
> +{
> +	u32 reg_val;
> +
> +	reg_val = readl_relaxed(reg);
> +	reg_val &= ~val;
> +	writel_relaxed(reg_val, reg);
> +
> +	/* Ensure above write is completed */
> +	mb();
> +}
> +
> +static void qcom_qusb2_phy_configure(void __iomem *base,
> +				struct qusb2_phy_init_tbl init_tbl[],
> +				int init_tbl_sz)
> +{
> +	int i;
> +
> +	for (i = 0; i < init_tbl_sz; i++) {
> +		writel_relaxed(init_tbl[i].cfg_val,
> +				base + init_tbl[i].reg_offset);
> +	}
> +
> +	/* flush buffered writes */
> +	mb();
> +}
> +
> +static void qusb2_phy_enable_clocks(struct qusb2_phy *qphy, bool on)

Maybe s/enable/toggle/ because we're not always enabling.

> +{
> +	if (on) {
> +		clk_prepare_enable(qphy->iface_clk);
> +		clk_prepare_enable(qphy->ref_clk_src);
> +	} else {
> +		clk_disable_unprepare(qphy->ref_clk_src);
> +		clk_disable_unprepare(qphy->iface_clk);
> +	}
> +
> +	dev_vdbg(&qphy->phy->dev, "%s(): clocks enabled\n", __func__);

Heh or disabled!

> +}
> +
> +static int qusb2_phy_enable_power(struct qusb2_phy *qphy, bool on)

Maybe s/enable/toggle/ because we're not always enabling.

> +{
> +	int ret;
> +	struct device *dev = &qphy->phy->dev;
> +
> +	if (!on)
> +		goto disable_vdda_phy_dpdm;
> +
> +	ret = regulator_enable(qphy->vdd_phy);
> +	if (ret) {
> +		dev_err(dev, "Unable to enable vdd-phy:%d\n", ret);
> +		goto err_vdd_phy;
> +	}
> +
> +	ret = regulator_enable(qphy->vdda_pll);
> +	if (ret) {
> +		dev_err(dev, "Unable to enable vdda-pll:%d\n", ret);
> +		goto disable_vdd_phy;
> +	}
> +
> +	ret = regulator_enable(qphy->vdda_phy_dpdm);
> +	if (ret) {
> +		dev_err(dev, "Unable to enable vdda-phy-dpdm:%d\n", ret);
> +		goto disable_vdda_pll;
> +	}
> +
> +	dev_vdbg(dev, "%s() regulators are turned on.\n", __func__);

Drop the full stop please.

> +
> +	return ret;
> +
> +disable_vdda_phy_dpdm:
> +	regulator_disable(qphy->vdda_phy_dpdm);
> +disable_vdda_pll:
> +	regulator_disable(qphy->vdda_pll);
> +disable_vdd_phy:
> +	regulator_disable(qphy->vdd_phy);
> +err_vdd_phy:
> +	dev_vdbg(dev, "%s() regulators are turned off.\n", __func__);
> +	return ret;
> +}
> +
> +/*
> + * Fetches HS Tx tuning value from e-fuse and sets QUSB2PHY_PORT_TUNE2
> + * register.
> + * For any error case, skip setting the value and use the default value.
> + */
> +static int qusb2_phy_set_tune2_param(struct qusb2_phy *qphy)
> +{
> +	struct device *dev = &qphy->phy->dev;
> +	struct nvmem_cell *cell;
> +	ssize_t len;
> +	u8 *val;
> +
> +	/*
> +	 * Read EFUSE register having TUNE2 parameter's high nibble.
> +	 * If efuse register shows value as 0x0, or if we fail to find
> +	 * a valid efuse register settings, then use default value
> +	 * as 0xB for high nibble that we have already set while
> +	 * configuring phy.
> +	 */
> +	cell = devm_nvmem_cell_get(dev, "tune2_hstx_trim_efuse");
> +	if (IS_ERR(cell)) {
> +		if (PTR_ERR(cell) == -EPROBE_DEFER)
> +			return PTR_ERR(cell);
> +		goto skip;

Why do we get the nvmem cell here? Wouldn't we want to get it
during probe? Returning probe defer here during init would be
odd.

> +	}
> +
> +	/*
> +	 * we need to read only one byte here, since the required
> +	 * parameter value fits in one nibble
> +	 */
> +	val = (u8 *)nvmem_cell_read(cell, &len);

Shouldn't need the cast here. Also it would be nice if
nvmem_cell_read() didn't require a second argument if we don't
care for it. We should update the API to allow NULL there.

> +	if (!IS_ERR(val)) {
> +		/* Fused TUNE2 value is the higher nibble only */
> +		qusb2_setbits(qphy->base + QUSB2PHY_PORT_TUNE2,
> +							val[0] << 0x4);
> +	} else {
> +		dev_dbg(dev, "failed reading hs-tx trim value: %ld\n",
> +							PTR_ERR(val));
> +	}
> +
> +skip:
> +	return 0;
> +}
> +
[...]
> +
> +static int qusb2_phy_init(struct phy *phy)
> +{
> +	struct qusb2_phy *qphy = phy_get_drvdata(phy);
> +	unsigned int reset_val;
> +	unsigned int clk_scheme;
> +	int ret;
> +
> +	dev_vdbg(&phy->dev, "Initializing QUSB2 phy\n");
> +
> +	/* enable ahb interface clock to program phy */
> +	clk_prepare_enable(qphy->cfg_ahb_clk);

What if that fails?

> +
> +	/* Perform phy reset */
> +	ret = reset_control_assert(qphy->phy_reset);
> +	if (ret) {
> +		dev_err(&phy->dev, "Failed to assert phy_reset\n");
> +		return ret;
> +	}
> +	/* 100 us delay to keep PHY in reset mode */
> +	usleep_range(100, 150);
> +	ret = reset_control_deassert(qphy->phy_reset);
> +	if (ret) {
> +		dev_err(&phy->dev, "Failed to de-assert phy_reset\n");
> +		return ret;
> +	}
> +
> +	/* Disable the PHY */
> +	qusb2_setbits(qphy->base + QUSB2PHY_PORT_POWERDOWN,
> +			CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
> +
> +	/* save reset value to override based on clk scheme */
> +	reset_val = readl_relaxed(qphy->base + QUSB2PHY_PLL_TEST);
> +
> +	qcom_qusb2_phy_configure(qphy->base, qphy->cfg->phy_init_tbl,
> +				qphy->cfg->phy_init_tbl_sz);
> +
> +	/* Check for efuse value for tuning the PHY */
> +	ret = qusb2_phy_set_tune2_param(qphy);
> +	if (ret)
> +		return ret;
> +
> +	/* Enable the PHY */
> +	qusb2_clrbits(qphy->base + QUSB2PHY_PORT_POWERDOWN, POWER_DOWN);
> +
> +	/* Require to get phy pll lock successfully */
> +	usleep_range(150, 160);
> +
> +	/* Default is Single-ended clock on msm8996 */
> +	qphy->has_se_clk_scheme = true;
> +	/*
> +	 * read TCSR_PHY_CLK_SCHEME register to check if Single-ended

Capital Single again?

> +	 * clock scheme is selected. If yes, then disable differential
> +	 * ref_clk and use single-ended clock, otherwise use differential
> +	 * ref_clk only.
> +	 */
> +	if (qphy->tcsr) {
> +		ret = regmap_read(qphy->tcsr, qphy->cfg->clk_scheme_offset,
> +							&clk_scheme);
> +		/* is it a differential clock scheme ? */
> +		if (!(clk_scheme & PHY_CLK_SCHEME_SEL)) {
> +			dev_vdbg(&phy->dev, "%s: select differential clk src\n",
> +								__func__);
> +			qphy->has_se_clk_scheme = false;
> +		} else {
> +			dev_vdbg(&phy->dev, "%s: select single-ended clk src\n",
> +								__func__);
> +		}
> +	}
> +
> +	if (!qphy->has_se_clk_scheme) {
> +		reset_val &= ~CLK_REF_SEL;
> +		clk_prepare_enable(qphy->ref_clk);

And if that fails?

> +	} else {
> +		reset_val |= CLK_REF_SEL;
> +	}
> +
> +	writel_relaxed(reset_val, qphy->base + QUSB2PHY_PLL_TEST);
> +
> +	/* Make sure that above write is completed to get PLL source clock */
> +	wmb();
> +
> +	/* Required to get PHY PLL lock successfully */
> +	usleep_range(100, 110);
> +
> +	if (!(readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS) &
> +					PLL_LOCKED)) {
> +		dev_err(&phy->dev, "QUSB PHY PLL LOCK fails:%x\n",
> +			readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS));

Would be pretty funny if this was locked now when the error
printk runs. Are there other bits in there that are helpful?

> +		return -EBUSY;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qusb2_phy_exit(struct phy *phy)
> +{
> +	struct qusb2_phy *qphy = phy_get_drvdata(phy);
> +
> +	/* Disable the PHY */
> +	qusb2_setbits(qphy->base + QUSB2PHY_PORT_POWERDOWN,
> +			CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
> +
> +	if (!qphy->has_se_clk_scheme)
> +		clk_disable_unprepare(qphy->ref_clk);
> +
> +	clk_disable_unprepare(qphy->cfg_ahb_clk);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops qusb2_phy_gen_ops = {
> +	.init		= qusb2_phy_init,
> +	.exit		= qusb2_phy_exit,
> +	.power_on	= qusb2_phy_poweron,
> +	.power_off	= qusb2_phy_poweroff,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static const struct of_device_id qusb2_phy_of_match_table[] = {
> +	{
> +		.compatible	= "qcom,msm8996-qusb2-phy",
> +		.data		= &msm8996_phy_init_cfg,
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, qusb2_phy_of_match_table);
> +
> +static int qusb2_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct qusb2_phy *qphy;
> +	struct phy_provider *phy_provider;
> +	struct phy *generic_phy;
> +	const struct of_device_id *match;
> +	struct resource *res;
> +	int ret;
> +
> +	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
> +	if (!qphy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	qphy->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(qphy->base))
> +		return PTR_ERR(qphy->base);
> +
> +	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb_clk");
> +	if (IS_ERR(qphy->cfg_ahb_clk)) {
> +		ret = PTR_ERR(qphy->cfg_ahb_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get cfg_ahb_clk\n");
> +		return ret;
> +	}
> +
> +	qphy->ref_clk_src = devm_clk_get(dev, "ref_clk_src");
> +	if (IS_ERR(qphy->ref_clk_src)) {
> +		ret = PTR_ERR(qphy->ref_clk_src);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "clk get failed for ref_clk_src\n");
> +		return ret;
> +	}
> +
> +	qphy->ref_clk = devm_clk_get(dev, "ref_clk");
> +	if (IS_ERR(qphy->ref_clk)) {
> +		ret = PTR_ERR(qphy->ref_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "clk get failed for ref_clk\n");
> +		return ret;
> +	} else {
> +		clk_set_rate(qphy->ref_clk, 19200000);

Drop the else. Also what if clk_set_rate() fails?

> +	}
> +
> +	qphy->iface_clk = devm_clk_get(dev, "iface_clk");
> +	if (IS_ERR(qphy->iface_clk)) {
> +		ret = PTR_ERR(qphy->iface_clk);
> +		if (ret != -EPROBE_DEFER) {
> +			qphy->iface_clk = NULL;
> +			dev_dbg(dev, "clk get failed for iface_clk\n");
> +		} else {
> +			return ret;
> +		}

		if (PTR_ERR(qphy->iface_clk) == -EPROBE_DEFER)
			return -EPROBE_DEFER;
		qphy->iface_clk = NULL;
		dev_dbg(dev, "clk get failed for iface_clk\n");

Is shorter.
		
> +	}
> +
> +	qphy->phy_reset = devm_reset_control_get(&pdev->dev, "phy");
> +	if (IS_ERR(qphy->phy_reset)) {
> +		dev_err(dev, "failed to get phy core reset\n");
> +		return PTR_ERR(qphy->phy_reset);
> +	}
> +
> +	qphy->vdd_phy = devm_regulator_get(dev, "vdd-phy");
> +	if (IS_ERR(qphy->vdd_phy)) {
> +		dev_err(dev, "unable to get vdd-phy supply\n");
> +		return PTR_ERR(qphy->vdd_phy);
> +	}
> +
> +	qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll");
> +	if (IS_ERR(qphy->vdda_pll)) {
> +		dev_err(dev, "unable to get vdda-pll supply\n");
> +		return PTR_ERR(qphy->vdda_pll);
> +	}
> +
> +	qphy->vdda_phy_dpdm = devm_regulator_get(dev, "vdda-phy-dpdm");
> +	if (IS_ERR(qphy->vdda_phy_dpdm)) {
> +		dev_err(dev, "unable to get vdda-phy-dpdm supply\n");
> +		return PTR_ERR(qphy->vdda_phy_dpdm);
> +	}
> +
> +	/* Get the specific init parameters of QMP phy */
> +	match = of_match_node(qusb2_phy_of_match_table, dev->of_node);
> +	qphy->cfg = match->data;

Use of_device_get_match_data() instead.

> +
> +	qphy->tcsr = syscon_regmap_lookup_by_phandle(dev->of_node,
> +							"qcom,tcsr-syscon");
> +	if (IS_ERR(qphy->tcsr)) {
> +		dev_dbg(dev, "Failed to lookup TCSR regmap\n");
> +		qphy->tcsr = NULL;
> +	}
> +
> +	generic_phy = devm_phy_create(dev, NULL, &qusb2_phy_gen_ops);
> +	if (IS_ERR(generic_phy)) {
> +		ret = PTR_ERR(generic_phy);
> +		dev_err(dev, "%s: failed to create phy %d\n", __func__, ret);
> +		return ret;
> +	}
> +	qphy->phy = generic_phy;
> +
> +	dev_set_drvdata(dev, qphy);
> +	phy_set_drvdata(generic_phy, qphy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider)) {
> +		ret = PTR_ERR(phy_provider);
> +		dev_err(dev, "%s: failed to register phy %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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

* [PATCH 6/6] ARM: dts: keystone: enable time synchronization (cpts) submodule
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko
In-Reply-To: <20161128230428.6872-1-grygorii.strashko@ti.com>

This patch adds DT configuration for Time Synchronization (CPTS) submodule
which is present on KeyStone 66AK2HK/E/Lx 1G Switch Subsystem.
The SYSCLK2 is selected as CPTS rftclk by default.
The ts_comp enabled for 66AK2E/L SoCs.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/keystone-k2e-netcp.dtsi  | 3 +++
 arch/arm/boot/dts/keystone-k2hk-netcp.dtsi | 1 +
 arch/arm/boot/dts/keystone-k2l-netcp.dtsi  | 3 +++
 3 files changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
index ba828cb..919e655 100644
--- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
@@ -158,6 +158,9 @@ netcp: netcp@24000000 {
 			/* enable-ale; */
 			tx-queue = <896>;
 			tx-channel = "nettx";
+			cpts-rftclk-sel = <0>;
+			cpts-ext-ts-inputs = <6>;
+			cpts-ts-comp-length;
 
 			interfaces {
 				gbe0: interface-0 {
diff --git a/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi b/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi
index a5ac845..772097b 100644
--- a/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi
@@ -177,6 +177,7 @@ netcp: netcp@2000000 {
 			/* enable-ale; */
 			tx-queue = <648>;
 			tx-channel = "nettx";
+			cpts-rftclk-sel = <0>;
 
 			interfaces {
 				gbe0: interface-0 {
diff --git a/arch/arm/boot/dts/keystone-k2l-netcp.dtsi b/arch/arm/boot/dts/keystone-k2l-netcp.dtsi
index b6f2682..580e2b2 100644
--- a/arch/arm/boot/dts/keystone-k2l-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2l-netcp.dtsi
@@ -157,6 +157,9 @@ netcp: netcp@26000000 {
 			/* enable-ale; */
 			tx-queue = <896>;
 			tx-channel = "nettx";
+			cpts-rftclk-sel = <0>;
+			cpts-ext-ts-inputs = <6>;
+			cpts-ts-comp-length;
 
 			interfaces {
 				gbe0: interface-0 {
-- 
2.10.1

^ permalink raw reply related

* [PATCH 5/6] ARM: keystone: dts: fix netcp clocks and add names
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
	Richard Cochran
  Cc: Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok,
	Grygorii Strashko
In-Reply-To: <20161128230428.6872-1-grygorii.strashko-l0cyMroinI0@public.gmane.org>

From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>

Fix the pa clock to point to the clkpa which has clock rate of 1/3 of PA
PLL clock and add clock names.

Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/keystone-k2e-netcp.dtsi  | 3 ++-
 arch/arm/boot/dts/keystone-k2hk-netcp.dtsi | 3 ++-
 arch/arm/boot/dts/keystone-k2l-netcp.dtsi  | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
index ac990f6..ba828cb 100644
--- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
@@ -138,7 +138,8 @@ netcp: netcp@24000000 {
 	/* NetCP address range */
 	ranges = <0 0x24000000 0x1000000>;
 
-	clocks = <&papllclk>, <&clkcpgmac>, <&chipclk12>;
+	clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+	clock-names = "pa_clk", "ethss_clk", "cpts";
 	dma-coherent;
 
 	ti,navigator-dmas = <&dma_gbe 0>,
diff --git a/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi b/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi
index f86d6dd..a5ac845 100644
--- a/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2hk-netcp.dtsi
@@ -155,7 +155,8 @@ netcp: netcp@2000000 {
 	/* NetCP address range */
 	ranges  = <0 0x2000000 0x100000>;
 
-	clocks = <&papllclk>, <&clkcpgmac>, <&chipclk12>;
+	clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+	clock-names = "pa_clk", "ethss_clk", "cpts";
 	dma-coherent;
 
 	ti,navigator-dmas = <&dma_gbe 22>,
diff --git a/arch/arm/boot/dts/keystone-k2l-netcp.dtsi b/arch/arm/boot/dts/keystone-k2l-netcp.dtsi
index 5acbd0d..b6f2682 100644
--- a/arch/arm/boot/dts/keystone-k2l-netcp.dtsi
+++ b/arch/arm/boot/dts/keystone-k2l-netcp.dtsi
@@ -137,7 +137,8 @@ netcp: netcp@26000000 {
 	/* NetCP address range */
 	ranges = <0 0x26000000 0x1000000>;
 
-	clocks = <&clkosr>, <&papllclk>, <&clkcpgmac>, <&chipclk12>;
+	clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>, <&clkosr>;
+	clock-names = "pa_clk", "ethss_clk", "cpts", "osr_clk";
 	dma-coherent;
 
 	ti,navigator-dmas = <&dma_gbe 0>,
-- 
2.10.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 related

* [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko
In-Reply-To: <20161128230428.6872-1-grygorii.strashko@ti.com>

The TS_COMP output in the CPSW CPTS module is asserted for
ts_comp_length[15:0] RCLK periods when the time_stamp value compares
with the ts_comp_val[31:0] and the length value is non-zero. The
TS_COMP pulse edge occurs three RCLK periods after the values
compare. A timestamp compare event is pushed into the event FIFO when
TS_COMP is asserted.

This patch adds support of Pulse-Per-Second (PPS) by using the
timestamp compare output. The CPTS driver adds one second of counter
value to the ts_comp_val register after each assertion of the TS_COMP
output. The TS_COMP pulse polarity and width are configurable in DT.

Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/net/keystone-netcp.txt     |  10 +
 drivers/net/ethernet/ti/cpts.c                     | 237 ++++++++++++++++++++-
 drivers/net/ethernet/ti/cpts.h                     |  14 +-
 3 files changed, 251 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
index 1c805319..060af96 100644
--- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -127,6 +127,16 @@ Optional properties:
 		The number of external time stamp channels.
 		The different CPTS versions might support up 8
 		external time stamp channels. if absent - unsupported.
+	- cpts-ts-comp-length:
+		Enable time stamp comparison event and TS_COMP signal output
+		generation when CPTS counter reaches a value written to
+		the TS_COMP_VAL register.
+		The generated pulse width is 3 refclk cycles if this property
+		has no value (empty) or, otherwise, it should specify desired
+		pulse width in number of refclk periods - max value 2^16.
+		TS_COMP functionality will be disabled if not present.
+	- cpts-ts-comp-polarity-low:
+		Set polarity of TS_COMP signal to low. Default is hight.
 
 NetCP interface properties: Interface specification for NetCP sub-modules.
 Required properties:
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 2f7641a..8ff70cc 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -31,9 +31,13 @@
 
 #include "cpts.h"
 
+#define CPTS_TS_COMP_PULSE_LENGTH_DEF	3
+
 #define cpts_read32(c, r)	readl_relaxed(&c->reg->r)
 #define cpts_write32(c, v, r)	writel_relaxed(v, &c->reg->r)
 
+static int cpts_report_ts_events(struct cpts *cpts, bool pps_reload);
+
 static int cpts_event_port(struct cpts_event *event)
 {
 	return (event->high >> PORT_NUMBER_SHIFT) & PORT_NUMBER_MASK;
@@ -108,6 +112,7 @@ static int cpts_fifo_read(struct cpts *cpts, int match)
 		type = event_type(event);
 		switch (type) {
 		case CPTS_EV_HW:
+		case CPTS_EV_COMP:
 			event->tmo +=
 				msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
 		case CPTS_EV_PUSH:
@@ -153,6 +158,60 @@ static cycle_t cpts_systim_read(const struct cyclecounter *cc)
 	return val;
 }
 
+static cycle_t cpts_cc_ns2cyc(struct cpts *cpts, u64 nsecs)
+{
+	cycle_t cyc = (nsecs << cpts->cc.shift) + nsecs;
+
+	do_div(cyc, cpts->cc.mult);
+
+	return cyc;
+}
+
+static void cpts_ts_comp_disable(struct cpts *cpts)
+{
+	cpts_write32(cpts, 0, ts_comp_length);
+}
+
+static void cpts_ts_comp_enable(struct cpts *cpts)
+{
+	/* TS_COMP_LENGTH should be 0 while the TS_COMP_VAL value is
+	 * being written
+	 */
+	cpts_write32(cpts, 0, ts_comp_length);
+	cpts_write32(cpts, cpts->ts_comp_next, ts_comp_val);
+	cpts_write32(cpts, cpts->ts_comp_length, ts_comp_length);
+}
+
+static void cpts_ts_comp_add_ns(struct cpts *cpts, s64 add_ns)
+{
+	cycle_t cyc_next;
+
+	if (add_ns == NSEC_PER_SEC)
+		/* avoid calculation */
+		cyc_next = cpts->ts_comp_one_sec_cycs;
+	else
+		cyc_next = cpts_cc_ns2cyc(cpts, add_ns);
+
+	cyc_next += cpts->ts_comp_next;
+	cpts->ts_comp_next = cyc_next & cpts->cc.mask;
+	pr_debug("cpts comp ts_comp_next: %u\n", cpts->ts_comp_next);
+}
+
+static void cpts_ts_comp_settime(struct cpts *cpts, s64 now_ns)
+{
+	struct timespec64 ts;
+
+	if (cpts->ts_comp_enabled) {
+		ts = ns_to_timespec64(now_ns);
+
+		/* align pulse to next sec boundary and add one sec */
+		cpts_ts_comp_add_ns(cpts, NSEC_PER_SEC - ts.tv_nsec);
+
+		/* enable ts_comp pulse */
+		cpts_ts_comp_enable(cpts);
+	}
+}
+
 /* PTP clock operations */
 
 static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
@@ -162,6 +221,7 @@ static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	int neg_adj = 0;
 	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
+	u64 ns;
 
 	if (ppb < 0) {
 		neg_adj = 1;
@@ -172,14 +232,31 @@ static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	adj *= ppb;
 	diff = div_u64(adj, 1000000000ULL);
 
+	mutex_lock(&cpts->ptp_clk_mutex);
+
 	spin_lock_irqsave(&cpts->lock, flags);
+	if (cpts->ts_comp_enabled) {
+		cpts_ts_comp_disable(cpts);
+		/* if any, report existing pulse before adj */
+		cpts_fifo_read(cpts, CPTS_EV_COMP);
+		/* if any, report existing pulse before adj */
+		cpts_report_ts_events(cpts, false);
+	}
 
 	timecounter_read(&cpts->tc);
 
 	cpts->cc.mult = neg_adj ? mult - diff : mult + diff;
-
+	/* get updated time with adj */
+	ns = timecounter_read(&cpts->tc);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
+	if (cpts->ts_comp_enabled)
+		cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
+	cpts_ts_comp_settime(cpts, ns);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	return 0;
 }
 
@@ -187,11 +264,28 @@ static int cpts_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
+	u64 ns;
+
+	mutex_lock(&cpts->ptp_clk_mutex);
 
 	spin_lock_irqsave(&cpts->lock, flags);
+	if (cpts->ts_comp_enabled) {
+		cpts_ts_comp_disable(cpts);
+		/* if any, report existing pulse before adj */
+		cpts_fifo_read(cpts, CPTS_EV_COMP);
+		/* if any, report existing pulse before adj */
+		cpts_report_ts_events(cpts, false);
+	}
+
 	timecounter_adjtime(&cpts->tc, delta);
+	ns = timecounter_read(&cpts->tc);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
+	cpts_ts_comp_settime(cpts, ns);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	return 0;
 }
 
@@ -213,25 +307,90 @@ static int cpts_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 static int cpts_ptp_settime(struct ptp_clock_info *ptp,
 			    const struct timespec64 *ts)
 {
-	u64 ns;
-	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
+	unsigned long flags;
+	u64 ns;
 
 	ns = timespec64_to_ns(ts);
 
+	mutex_lock(&cpts->ptp_clk_mutex);
+
 	spin_lock_irqsave(&cpts->lock, flags);
+	if (cpts->ts_comp_enabled) {
+		cpts_ts_comp_disable(cpts);
+		/* if any, get existing pulse event before adj */
+		cpts_fifo_read(cpts, CPTS_EV_COMP);
+		/* if any, report existing pulse before adj */
+		cpts_report_ts_events(cpts, false);
+	}
+
 	timecounter_init(&cpts->tc, &cpts->cc, ns);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
+	cpts_ts_comp_settime(cpts, ns);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	return 0;
 }
 
-static int cpts_report_ts_events(struct cpts *cpts)
+static int cpts_pps_enable(struct cpts *cpts, int on)
+{
+	struct timespec64 ts;
+	unsigned long flags;
+	u64 ns;
+
+	if (cpts->ts_comp_enabled == on)
+		return 0;
+
+	mutex_lock(&cpts->ptp_clk_mutex);
+	cpts->ts_comp_enabled = on;
+
+	if (!on) {
+		cpts_ts_comp_disable(cpts);
+		if (!cpts->hw_ts_enable)
+			cpts->ov_check_period = cpts->ov_check_period_slow;
+		mutex_unlock(&cpts->ptp_clk_mutex);
+		return 0;
+	}
+
+	/* get current counter value */
+	spin_lock_irqsave(&cpts->lock, flags);
+	ns = timecounter_read(&cpts->tc);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
+	spin_unlock_irqrestore(&cpts->lock, flags);
+
+	ts = ns_to_timespec64(ns);
+	cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
+	/* align to next sec boundary and add one sec to avoid the situation
+	 * when the current time is very close to the next second point and
+	 * it might be possible that ts_comp_val will be configured to
+	 * the time in the past.
+	 */
+	cpts_ts_comp_add_ns(cpts, 2 * NSEC_PER_SEC - ts.tv_nsec);
+
+	/* enable ts_comp pulse */
+	cpts_ts_comp_enable(cpts);
+
+	/* poll for events faster - evry 200 ms */
+	cpts->ov_check_period = msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
+
+	mod_delayed_work(system_wq, &cpts->overflow_work,
+			 cpts->ov_check_period);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
+	return 0;
+}
+
+static int cpts_report_ts_events(struct cpts *cpts, bool pps_reload)
 {
 	struct list_head *this, *next;
 	struct ptp_clock_event pevent;
 	struct cpts_event *event;
 	int reported = 0, ev;
+	u64 ns;
 
 	list_for_each_safe(this, next, &cpts->events) {
 		event = list_entry(this, struct cpts_event, list);
@@ -248,6 +407,33 @@ static int cpts_report_ts_events(struct cpts *cpts)
 			++reported;
 			continue;
 		}
+
+		if (event_type(event) == CPTS_EV_COMP) {
+			list_del_init(&event->list);
+			list_add(&event->list, &cpts->pool);
+			if (cpts->ts_comp_next != event->low) {
+				pr_err("cpts ts_comp mismatch: %08x %08x\n",
+				       cpts->ts_comp_next, event->low);
+				continue;
+			} else
+				pr_debug("cpts comp ev tstamp: %u\n",
+					 event->low);
+
+			/* report the event */
+			ns = timecounter_cyc2time(&cpts->tc, event->low);
+			pevent.type = PTP_CLOCK_PPSUSR;
+			pevent.pps_times.ts_real = ns_to_timespec64(ns);
+			ptp_clock_event(cpts->clock, &pevent);
+
+			if (pps_reload) {
+				/* reload: add ns to ts_comp */
+				cpts_ts_comp_add_ns(cpts, NSEC_PER_SEC);
+				/* enable ts_comp pulse with new val */
+				cpts_ts_comp_enable(cpts);
+			}
+			++reported;
+			continue;
+		}
 	}
 	return reported;
 }
@@ -264,6 +450,8 @@ static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
 	if (((cpts->hw_ts_enable & BIT(index)) >> index) == on)
 		return 0;
 
+	mutex_lock(&cpts->ptp_clk_mutex);
+
 	spin_lock_irqsave(&cpts->lock, flags);
 
 	v = cpts_read32(cpts, control);
@@ -282,12 +470,12 @@ static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
 		/* poll for events faster - evry 200 ms */
 		cpts->ov_check_period =
 			msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
-	else
+	else if (!cpts->ts_comp_enabled)
 		cpts->ov_check_period = cpts->ov_check_period_slow;
 
 	mod_delayed_work(system_wq, &cpts->overflow_work,
 			 cpts->ov_check_period);
-
+	mutex_unlock(&cpts->ptp_clk_mutex);
 	return 0;
 }
 
@@ -299,6 +487,8 @@ static int cpts_ptp_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		return cpts_extts_enable(cpts, rq->extts.index, on);
+	case PTP_CLK_REQ_PPS:
+		return cpts_pps_enable(cpts, on);
 	default:
 		break;
 	}
@@ -326,12 +516,15 @@ static void cpts_overflow_check(struct work_struct *work)
 	struct timespec64 ts;
 	unsigned long flags;
 
+	mutex_lock(&cpts->ptp_clk_mutex);
 	spin_lock_irqsave(&cpts->lock, flags);
 	ts = ns_to_timespec64(timecounter_read(&cpts->tc));
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
-	if (cpts->hw_ts_enable)
-		cpts_report_ts_events(cpts);
+	if (cpts->hw_ts_enable || cpts->ts_comp_enabled)
+		cpts_report_ts_events(cpts, true);
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
 	schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
 }
@@ -445,6 +638,7 @@ EXPORT_SYMBOL_GPL(cpts_tx_timestamp);
 int cpts_register(struct cpts *cpts)
 {
 	int err, i;
+	u32 control;
 
 	INIT_LIST_HEAD(&cpts->events);
 	INIT_LIST_HEAD(&cpts->pool);
@@ -453,7 +647,14 @@ int cpts_register(struct cpts *cpts)
 
 	clk_enable(cpts->refclk);
 
-	cpts_write32(cpts, CPTS_EN, control);
+	control = CPTS_EN;
+	if (cpts->caps & CPTS_CAP_TS_COMP_EN) {
+		if (cpts->caps & CPTS_CAP_TS_COMP_POL_LOW_SEL)
+			control &= ~TS_COMP_POL;
+		else
+			control |= TS_COMP_POL;
+	}
+	cpts_write32(cpts, control, control);
 	cpts_write32(cpts, TS_PEND_EN, int_enable);
 
 	cpts->cc.mult = cpts->cc_mult;
@@ -558,6 +759,20 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
 		cpts->rftclk_sel = prop & CPTS_RFTCLK_SEL_MASK;
 	}
 
+	if (of_property_read_bool(node, "cpts-ts-comp-length")) {
+		cpts->caps |= CPTS_CAP_TS_COMP_EN;
+		cpts->ts_comp_length = CPTS_TS_COMP_PULSE_LENGTH_DEF;
+	}
+
+	if (cpts->caps & CPTS_CAP_TS_COMP_EN) {
+		ret = of_property_read_u32(node, "cpts-ts-comp-length", &prop);
+		if (!ret)
+			cpts->ts_comp_length = prop;
+
+		if (of_property_read_bool(node, "cpts-ts-comp-polarity-low"))
+			cpts->caps |= CPTS_CAP_TS_COMP_POL_LOW_SEL;
+	}
+
 	if (!of_property_read_u32(node, "cpts-ext-ts-inputs", &prop))
 		cpts->ext_ts_inputs = prop;
 
@@ -584,6 +799,7 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	cpts->dev = dev;
 	cpts->reg = (struct cpsw_cpts __iomem *)regs;
 	spin_lock_init(&cpts->lock);
+	mutex_init(&cpts->ptp_clk_mutex);
 	INIT_DELAYED_WORK(&cpts->overflow_work, cpts_overflow_check);
 
 	ret = cpts_of_parse(cpts, node);
@@ -608,6 +824,9 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	if (cpts->ext_ts_inputs)
 		cpts->info.n_ext_ts = cpts->ext_ts_inputs;
 
+	if (cpts->caps & CPTS_CAP_TS_COMP_EN)
+		cpts->info.pps = 1;
+
 	cpts_calc_mult_shift(cpts);
 
 	return cpts;
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index ad80c95..a82520d 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -39,7 +39,8 @@ struct cpsw_cpts {
 	u32 ts_push;              /* Time stamp event push */
 	u32 ts_load_val;          /* Time stamp load value */
 	u32 ts_load_en;           /* Time stamp load enable */
-	u32 res2[2];
+	u32 ts_comp_val;          /* Time stamp comparison value, v1.5 & up */
+	u32 ts_comp_length;       /* Time stamp comp assert len, v1.5 & up */
 	u32 intstat_raw;          /* Time sync interrupt status raw */
 	u32 intstat_masked;       /* Time sync interrupt status masked */
 	u32 int_enable;           /* Time sync interrupt enable */
@@ -64,11 +65,14 @@ struct cpsw_cpts {
 #define HW3_TS_PUSH_EN       (1<<10) /* Hardware push 3 enable */
 #define HW2_TS_PUSH_EN       (1<<9)  /* Hardware push 2 enable */
 #define HW1_TS_PUSH_EN       (1<<8)  /* Hardware push 1 enable */
+#define TS_COMP_POL	     BIT(2)  /* TS_COMP Polarity */
 #define INT_TEST             (1<<1)  /* Interrupt Test */
 #define CPTS_EN              (1<<0)  /* Time Sync Enable */
 
 #define CPTS_RFTCLK_SEL_MASK 0x1f
 
+#define CPTS_TS_COMP_LENGTH_MASK 0xffff
+
 /*
  * Definitions for the single bit resisters:
  * TS_PUSH TS_LOAD_EN  INTSTAT_RAW INTSTAT_MASKED INT_ENABLE EVENT_POP
@@ -97,6 +101,7 @@ enum {
 	CPTS_EV_HW,   /* Hardware Time Stamp Push Event */
 	CPTS_EV_RX,   /* Ethernet Receive Event */
 	CPTS_EV_TX,   /* Ethernet Transmit Event */
+	CPTS_EV_COMP, /* Time Stamp Compare Event */
 };
 
 #define CPTS_FIFO_DEPTH 16
@@ -113,6 +118,8 @@ struct cpts_event {
 };
 
 #define CPTS_CAP_RFTCLK_SEL BIT(0)
+#define CPTS_CAP_TS_COMP_EN BIT(1)
+#define CPTS_CAP_TS_COMP_POL_LOW_SEL BIT(2)
 
 struct cpts {
 	struct device *dev;
@@ -137,6 +144,11 @@ struct cpts {
 	u32 ext_ts_inputs;
 	u32 hw_ts_enable;
 	u32 caps;
+	u32 ts_comp_next;	/* next time_stamp value to compare with */
+	u32 ts_comp_length;	/* TS_COMP Output pulse width */
+	u32 ts_comp_one_sec_cycs; /* number of counter cycles in one sec */
+	int ts_comp_enabled;
+	struct mutex ptp_clk_mutex; /* sync PTP interface with overflow_work */
 };
 
 void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
-- 
2.10.1

^ permalink raw reply related

* [PATCH 3/6] net: ethernet: ti: cpts: add support of cpts HW_TS_PUSH
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko
In-Reply-To: <20161128230428.6872-1-grygorii.strashko@ti.com>

This patch adds support of the CPTS HW_TS_PUSH events which are generated
by external low frequency time stamp channels on TI's OMAP CPSW and
Keystone 2 platforms. It supports up to 8 external time stamp channels for
HW_TS_PUSH input pins (the number of supported channel is different for
different SoCs and CPTS versions, check corresponding Data maual before
enabling it). Therefore, new DT property "cpts-ext-ts-inputs" is introduced
for specifying number of available external timestamp channels.

The PTP external timestamp (extts) infrastructure can be used for
HW_TS_PUSH timestamp controlling and reporting.

This also change overflow polling period when HW_TS_PUSH feature is
enabled - overflow check work will be scheduled more often (every
200ms) for proper HW_TS_PUSH events reporting.

Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt     |   4 +
 .../devicetree/bindings/net/keystone-netcp.txt     |   4 +
 drivers/net/ethernet/ti/cpts.c                     | 104 ++++++++++++++++++++-
 drivers/net/ethernet/ti/cpts.h                     |   6 ++
 4 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index ebda7c9..86a2f61 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -38,6 +38,10 @@ Optional properties:
 			  Mult and shift will be calculated basing on CPTS
 			  rftclk frequency if both cpts_clock_shift and
 			  cpts_clock_mult properties are not provided.
+- cpts-ext-ts-inputs	: The number of external time stamp channels.
+			  The different CPTS versions might support up 8
+			  external time stamp channels.
+			  if absent - unsupported.
 
 Slave Properties:
 Required properties:
diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
index ec4a241..1c805319 100644
--- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -123,6 +123,10 @@ Optional properties:
 
 		Defaults: clock_mult, clock_shift = calculated from
 		CPTS refclk
+	- cpts-ext-ts-inputs:
+		The number of external time stamp channels.
+		The different CPTS versions might support up 8
+		external time stamp channels. if absent - unsupported.
 
 NetCP interface properties: Interface specification for NetCP sub-modules.
 Required properties:
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 9c5b835..2f7641a 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -34,6 +34,11 @@
 #define cpts_read32(c, r)	readl_relaxed(&c->reg->r)
 #define cpts_write32(c, v, r)	writel_relaxed(v, &c->reg->r)
 
+static int cpts_event_port(struct cpts_event *event)
+{
+	return (event->high >> PORT_NUMBER_SHIFT) & PORT_NUMBER_MASK;
+}
+
 static int event_expired(struct cpts_event *event)
 {
 	return time_after(jiffies, event->tmo);
@@ -96,11 +101,15 @@ static int cpts_fifo_read(struct cpts *cpts, int match)
 		}
 
 		event = list_first_entry(&cpts->pool, struct cpts_event, list);
-		event->tmo = jiffies + 2;
+		event->tmo = jiffies +
+			     msecs_to_jiffies(CPTS_EVENT_RX_TX_TIMEOUT);
 		event->high = hi;
 		event->low = lo;
 		type = event_type(event);
 		switch (type) {
+		case CPTS_EV_HW:
+			event->tmo +=
+				msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
 		case CPTS_EV_PUSH:
 		case CPTS_EV_RX:
 		case CPTS_EV_TX:
@@ -109,7 +118,6 @@ static int cpts_fifo_read(struct cpts *cpts, int match)
 			break;
 		case CPTS_EV_ROLL:
 		case CPTS_EV_HALF:
-		case CPTS_EV_HW:
 			break;
 		default:
 			pr_err("cpts: unknown event type\n");
@@ -218,9 +226,83 @@ static int cpts_ptp_settime(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int cpts_report_ts_events(struct cpts *cpts)
+{
+	struct list_head *this, *next;
+	struct ptp_clock_event pevent;
+	struct cpts_event *event;
+	int reported = 0, ev;
+
+	list_for_each_safe(this, next, &cpts->events) {
+		event = list_entry(this, struct cpts_event, list);
+		ev = event_type(event);
+		if (ev == CPTS_EV_HW) {
+			list_del_init(&event->list);
+			list_add(&event->list, &cpts->pool);
+			/* report the event */
+			pevent.timestamp =
+				timecounter_cyc2time(&cpts->tc, event->low);
+			pevent.type = PTP_CLOCK_EXTTS;
+			pevent.index = cpts_event_port(event) - 1;
+			ptp_clock_event(cpts->clock, &pevent);
+			++reported;
+			continue;
+		}
+	}
+	return reported;
+}
+
+/* HW TS */
+static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
+{
+	unsigned long flags;
+	u32 v;
+
+	if (index >= cpts->info.n_ext_ts)
+		return -ENXIO;
+
+	if (((cpts->hw_ts_enable & BIT(index)) >> index) == on)
+		return 0;
+
+	spin_lock_irqsave(&cpts->lock, flags);
+
+	v = cpts_read32(cpts, control);
+	if (on) {
+		v |= BIT(8 + index);
+		cpts->hw_ts_enable |= BIT(index);
+	} else {
+		v &= ~BIT(8 + index);
+		cpts->hw_ts_enable &= ~BIT(index);
+	}
+	cpts_write32(cpts, v, control);
+
+	spin_unlock_irqrestore(&cpts->lock, flags);
+
+	if (cpts->hw_ts_enable)
+		/* poll for events faster - evry 200 ms */
+		cpts->ov_check_period =
+			msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
+	else
+		cpts->ov_check_period = cpts->ov_check_period_slow;
+
+	mod_delayed_work(system_wq, &cpts->overflow_work,
+			 cpts->ov_check_period);
+
+	return 0;
+}
+
 static int cpts_ptp_enable(struct ptp_clock_info *ptp,
 			   struct ptp_clock_request *rq, int on)
 {
+	struct cpts *cpts = container_of(ptp, struct cpts, info);
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_EXTTS:
+		return cpts_extts_enable(cpts, rq->extts.index, on);
+	default:
+		break;
+	}
+
 	return -EOPNOTSUPP;
 }
 
@@ -240,10 +322,16 @@ static struct ptp_clock_info cpts_info = {
 
 static void cpts_overflow_check(struct work_struct *work)
 {
-	struct timespec64 ts;
 	struct cpts *cpts = container_of(work, struct cpts, overflow_work.work);
+	struct timespec64 ts;
+	unsigned long flags;
 
-	cpts_ptp_gettime(&cpts->info, &ts);
+	spin_lock_irqsave(&cpts->lock, flags);
+	ts = ns_to_timespec64(timecounter_read(&cpts->tc));
+	spin_unlock_irqrestore(&cpts->lock, flags);
+
+	if (cpts->hw_ts_enable)
+		cpts_report_ts_events(cpts);
 	pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
 	schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
 }
@@ -422,6 +510,8 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
 
 	/* Calc overflow check period (maxsec / 2) */
 	cpts->ov_check_period = (HZ * maxsec) / 2;
+	cpts->ov_check_period_slow = cpts->ov_check_period;
+
 	dev_info(cpts->dev, "cpts: overflow check period %lu (jiffies)\n",
 		 cpts->ov_check_period);
 
@@ -468,6 +558,9 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
 		cpts->rftclk_sel = prop & CPTS_RFTCLK_SEL_MASK;
 	}
 
+	if (!of_property_read_u32(node, "cpts-ext-ts-inputs", &prop))
+		cpts->ext_ts_inputs = prop;
+
 	return 0;
 
 of_error:
@@ -512,6 +605,9 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	cpts->cc.mask = CLOCKSOURCE_MASK(32);
 	cpts->info = cpts_info;
 
+	if (cpts->ext_ts_inputs)
+		cpts->info.n_ext_ts = cpts->ext_ts_inputs;
+
 	cpts_calc_mult_shift(cpts);
 
 	return cpts;
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index c934b61..ad80c95 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -102,6 +102,9 @@ enum {
 #define CPTS_FIFO_DEPTH 16
 #define CPTS_MAX_EVENTS 32
 
+#define CPTS_EVENT_RX_TX_TIMEOUT 20 /* ms */
+#define CPTS_EVENT_HWSTAMP_TIMEOUT 200 /* ms */
+
 struct cpts_event {
 	struct list_head list;
 	unsigned long tmo;
@@ -129,7 +132,10 @@ struct cpts {
 	struct list_head pool;
 	struct cpts_event pool_data[CPTS_MAX_EVENTS];
 	unsigned long ov_check_period;
+	unsigned long ov_check_period_slow;
 	u32 rftclk_sel;
+	u32 ext_ts_inputs;
+	u32 hw_ts_enable;
 	u32 caps;
 };
 
-- 
2.10.1

^ permalink raw reply related

* [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko
In-Reply-To: <20161128230428.6872-1-grygorii.strashko@ti.com>

Some CPTS instances, which can be found on KeyStone 2 1/10G Ethernet
Switch Subsystems, can control an external multiplexer that selects
one of up to 32 clocks for time sync reference (RFTCLK). This feature
can be configured through CPTS_RFTCLK_SEL register (offset: x08).

Hence, introduce optional DT cpts_rftclk_sel poperty wich, if present,
will specify CPTS reference clock. The cpts_rftclk_sel should be
omitted in DT if HW doesn't support this feature. The external fixed
rate clocks can be defined in board files as "fixed-clock".

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/keystone-netcp.txt |  2 ++
 drivers/net/ethernet/ti/cpts.c                           | 12 ++++++++++++
 drivers/net/ethernet/ti/cpts.h                           |  8 +++++++-
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
index c37b54e..ec4a241 100644
--- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -114,6 +114,8 @@ Optional properties:
 				driver to them if needed.
 
  Properties related to cpts configurations.
+	- cpts-rftclk-sel: selects one of up to 32 clocks for time sync
+		reference.  Default = 0.
 	- cpts_clock_mult/cpts_clock_shift:
 		used for converting time counter cycles to ns as in
 
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index c96a94a..9c5b835 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -459,6 +459,15 @@ static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
 	    (!cpts->cc_mult && cpts->cc.shift))
 		goto of_error;
 
+	if (!of_property_read_u32(node, "cpts-rftclk-sel", &prop)) {
+		if (prop & ~CPTS_RFTCLK_SEL_MASK) {
+			dev_err(cpts->dev, "cpts: invalid cpts_rftclk_sel.\n");
+			goto of_error;
+		}
+		cpts->caps |= CPTS_CAP_RFTCLK_SEL;
+		cpts->rftclk_sel = prop & CPTS_RFTCLK_SEL_MASK;
+	}
+
 	return 0;
 
 of_error:
@@ -496,6 +505,9 @@ struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 
 	clk_prepare(cpts->refclk);
 
+	if (cpts->caps & CPTS_CAP_RFTCLK_SEL)
+		cpts_write32(cpts, cpts->rftclk_sel, rftclk_sel);
+
 	cpts->cc.read = cpts_systim_read;
 	cpts->cc.mask = CLOCKSOURCE_MASK(32);
 	cpts->info = cpts_info;
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index c96eca2..c934b61 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -35,7 +35,7 @@
 struct cpsw_cpts {
 	u32 idver;                /* Identification and version */
 	u32 control;              /* Time sync control */
-	u32 res1;
+	u32 rftclk_sel;		  /* Reference Clock Select Register */
 	u32 ts_push;              /* Time stamp event push */
 	u32 ts_load_val;          /* Time stamp load value */
 	u32 ts_load_en;           /* Time stamp load enable */
@@ -67,6 +67,8 @@ struct cpsw_cpts {
 #define INT_TEST             (1<<1)  /* Interrupt Test */
 #define CPTS_EN              (1<<0)  /* Time Sync Enable */
 
+#define CPTS_RFTCLK_SEL_MASK 0x1f
+
 /*
  * Definitions for the single bit resisters:
  * TS_PUSH TS_LOAD_EN  INTSTAT_RAW INTSTAT_MASKED INT_ENABLE EVENT_POP
@@ -107,6 +109,8 @@ struct cpts_event {
 	u32 low;
 };
 
+#define CPTS_CAP_RFTCLK_SEL BIT(0)
+
 struct cpts {
 	struct device *dev;
 	struct cpsw_cpts __iomem *reg;
@@ -125,6 +129,8 @@ struct cpts {
 	struct list_head pool;
 	struct cpts_event pool_data[CPTS_MAX_EVENTS];
 	unsigned long ov_check_period;
+	u32 rftclk_sel;
+	u32 caps;
 };
 
 void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
-- 
2.10.1

^ permalink raw reply related

* [PATCH 1/6] net: ethernet: ti: netcp: add support of cpts
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko
In-Reply-To: <20161128230428.6872-1-grygorii.strashko@ti.com>

From: WingMan Kwok <w-kwok2@ti.com>

This patch adds support of the cpts device found in the
gbe and 10gbe ethernet switches on the keystone 2 SoCs
(66AK2E/L/Hx, 66AK2Gx).

Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/net/keystone-netcp.txt     |   9 +
 drivers/net/ethernet/ti/Kconfig                    |   7 +-
 drivers/net/ethernet/ti/netcp.h                    |   2 +-
 drivers/net/ethernet/ti/netcp_core.c               |  18 +-
 drivers/net/ethernet/ti/netcp_ethss.c              | 437 ++++++++++++++++++++-
 5 files changed, 459 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
index 04ba1dc..c37b54e 100644
--- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -113,6 +113,15 @@ Optional properties:
 				will only initialize these ports and attach PHY
 				driver to them if needed.
 
+ Properties related to cpts configurations.
+	- cpts_clock_mult/cpts_clock_shift:
+		used for converting time counter cycles to ns as in
+
+		ns = (cycles * clock_mult) >> _shift
+
+		Defaults: clock_mult, clock_shift = calculated from
+		CPTS refclk
+
 NetCP interface properties: Interface specification for NetCP sub-modules.
 Required properties:
 - rx-channel:	the navigator packet dma channel name for rx.
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index ff7f518..dc217fd 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -75,12 +75,13 @@ config TI_CPSW
 
 config TI_CPTS
 	tristate "TI Common Platform Time Sync (CPTS) Support"
-	depends on TI_CPSW
+	depends on TI_CPSW || TI_KEYSTONE_NETCP
 	select PTP_1588_CLOCK
 	---help---
 	  This driver supports the Common Platform Time Sync unit of
-	  the CPSW Ethernet Switch. The unit can time stamp PTP UDP/IPv4
-	  and Layer 2 packets, and the driver offers a PTP Hardware Clock.
+	  the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem.
+	  The unit can time stamp PTP UDP/IPv4 and Layer 2 packets, and the
+	  driver offers a PTP Hardware Clock.
 
 config TI_KEYSTONE_NETCP
 	tristate "TI Keystone NETCP Core Support"
diff --git a/drivers/net/ethernet/ti/netcp.h b/drivers/net/ethernet/ti/netcp.h
index 17a26a4..0f58c58 100644
--- a/drivers/net/ethernet/ti/netcp.h
+++ b/drivers/net/ethernet/ti/netcp.h
@@ -121,7 +121,7 @@ struct netcp_packet {
 	bool			rxtstamp_complete;
 	void			*ts_context;
 
-	int	(*txtstamp_complete)(void *ctx, struct netcp_packet *pkt);
+	void (*txtstamp)(void *ctx, struct sk_buff *skb);
 };
 
 static inline u32 *netcp_push_psdata(struct netcp_packet *p_info,
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 3251666..a740e60 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -100,6 +100,11 @@ struct netcp_intf_modpriv {
 	void			*module_priv;
 };
 
+struct netcp_tx_cb {
+	void	*ts_context;
+	void	(*txtstamp)(void *context, struct sk_buff *skb);
+};
+
 static LIST_HEAD(netcp_devices);
 static LIST_HEAD(netcp_modules);
 static DEFINE_MUTEX(netcp_modules_lock);
@@ -730,6 +735,7 @@ static int netcp_process_one_rx_packet(struct netcp_intf *netcp)
 
 	/* Call each of the RX hooks */
 	p_info.skb = skb;
+	skb->dev = netcp->ndev;
 	p_info.rxtstamp_complete = false;
 	list_for_each_entry(rx_hook, &netcp->rxhook_list_head, list) {
 		int ret;
@@ -987,6 +993,7 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
 					  unsigned int budget)
 {
 	struct knav_dma_desc *desc;
+	struct netcp_tx_cb *tx_cb;
 	struct sk_buff *skb;
 	unsigned int dma_sz;
 	dma_addr_t dma;
@@ -1014,6 +1021,10 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
 			continue;
 		}
 
+		tx_cb = (struct netcp_tx_cb *)skb->cb;
+		if (tx_cb->txtstamp)
+			tx_cb->txtstamp(tx_cb->ts_context, skb);
+
 		if (netif_subqueue_stopped(netcp->ndev, skb) &&
 		    netif_running(netcp->ndev) &&
 		    (knav_pool_count(netcp->tx_pool) >
@@ -1154,6 +1165,7 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp,
 	struct netcp_tx_pipe *tx_pipe = NULL;
 	struct netcp_hook_list *tx_hook;
 	struct netcp_packet p_info;
+	struct netcp_tx_cb *tx_cb;
 	unsigned int dma_sz;
 	dma_addr_t dma;
 	u32 tmp = 0;
@@ -1164,7 +1176,7 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp,
 	p_info.tx_pipe = NULL;
 	p_info.psdata_len = 0;
 	p_info.ts_context = NULL;
-	p_info.txtstamp_complete = NULL;
+	p_info.txtstamp = NULL;
 	p_info.epib = desc->epib;
 	p_info.psdata = (u32 __force *)desc->psdata;
 	memset(p_info.epib, 0, KNAV_DMA_NUM_EPIB_WORDS * sizeof(__le32));
@@ -1189,6 +1201,10 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp,
 		goto out;
 	}
 
+	tx_cb = (struct netcp_tx_cb *)skb->cb;
+	tx_cb->ts_context = p_info.ts_context;
+	tx_cb->txtstamp = p_info.txtstamp;
+
 	/* update descriptor */
 	if (p_info.psdata_len) {
 		/* psdata points to both native-endian and device-endian data */
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index d543298..4856521 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -23,10 +23,13 @@
 #include <linux/of_mdio.h>
 #include <linux/of_address.h>
 #include <linux/if_vlan.h>
+#include <linux/ptp_classify.h>
+#include <linux/net_tstamp.h>
 #include <linux/ethtool.h>
 
 #include "cpsw_ale.h"
 #include "netcp.h"
+#include "cpts.h"
 
 #define NETCP_DRIVER_NAME		"TI KeyStone Ethernet Driver"
 #define NETCP_DRIVER_VERSION		"v1.0"
@@ -51,6 +54,7 @@
 #define GBE13_EMAC_OFFSET		0x100
 #define GBE13_SLAVE_PORT2_OFFSET	0x200
 #define GBE13_HW_STATS_OFFSET		0x300
+#define GBE13_CPTS_OFFSET		0x500
 #define GBE13_ALE_OFFSET		0x600
 #define GBE13_HOST_PORT_NUM		0
 #define GBE13_NUM_ALE_ENTRIES		1024
@@ -74,6 +78,7 @@
 #define GBENU_SLAVE_PORT_OFFSET		0x2000
 #define GBENU_EMAC_OFFSET		0x2330
 #define GBENU_HW_STATS_OFFSET		0x1a000
+#define GBENU_CPTS_OFFSET		0x1d000
 #define GBENU_ALE_OFFSET		0x1e000
 #define GBENU_HOST_PORT_NUM		0
 #define GBENU_NUM_ALE_ENTRIES		1024
@@ -93,6 +98,7 @@
 #define XGBE10_HOST_PORT_OFFSET		0x34
 #define XGBE10_SLAVE_PORT_OFFSET	0x64
 #define XGBE10_EMAC_OFFSET		0x400
+#define XGBE10_CPTS_OFFSET		0x600
 #define XGBE10_ALE_OFFSET		0x700
 #define XGBE10_HW_STATS_OFFSET		0x800
 #define XGBE10_HOST_PORT_NUM		0
@@ -155,6 +161,7 @@
 
 #define GBE_TX_QUEUE				648
 #define	GBE_TXHOOK_ORDER			0
+#define	GBE_RXHOOK_ORDER			0
 #define GBE_DEFAULT_ALE_AGEOUT			30
 #define SLAVE_LINK_IS_XGMII(s) ((s)->link_interface >= XGMII_LINK_MAC_PHY)
 #define NETCP_LINK_STATE_INVALID		-1
@@ -169,6 +176,56 @@
 
 #define HOST_TX_PRI_MAP_DEFAULT			0x00000000
 
+#if IS_ENABLED(CONFIG_TI_CPTS)
+/* Px_TS_CTL register fields */
+#define TS_RX_ANX_F_EN				BIT(0)
+#define TS_RX_VLAN_LT1_EN			BIT(1)
+#define TS_RX_VLAN_LT2_EN			BIT(2)
+#define TS_RX_ANX_D_EN				BIT(3)
+#define TS_TX_ANX_F_EN				BIT(4)
+#define TS_TX_VLAN_LT1_EN			BIT(5)
+#define TS_TX_VLAN_LT2_EN			BIT(6)
+#define TS_TX_ANX_D_EN				BIT(7)
+#define TS_LT2_EN				BIT(8)
+#define TS_RX_ANX_E_EN				BIT(9)
+#define TS_TX_ANX_E_EN				BIT(10)
+#define TS_MSG_TYPE_EN_SHIFT			16
+#define TS_MSG_TYPE_EN_MASK			0xffff
+
+/* Px_TS_SEQ_LTYPE register fields */
+#define TS_SEQ_ID_OFS_SHIFT			16
+#define TS_SEQ_ID_OFS_MASK			0x3f
+
+/* Px_TS_CTL_LTYPE2 register fields */
+#define TS_107					BIT(16)
+#define TS_129					BIT(17)
+#define TS_130					BIT(18)
+#define TS_131					BIT(19)
+#define TS_132					BIT(20)
+#define TS_319					BIT(21)
+#define TS_320					BIT(22)
+#define TS_TTL_NONZERO				BIT(23)
+#define TS_UNI_EN				BIT(24)
+#define TS_UNI_EN_SHIFT				24
+
+#define TS_TX_ANX_ALL_EN	 \
+	(TS_TX_ANX_D_EN	| TS_TX_ANX_E_EN | TS_TX_ANX_F_EN)
+
+#define TS_RX_ANX_ALL_EN	 \
+	(TS_RX_ANX_D_EN	| TS_RX_ANX_E_EN | TS_RX_ANX_F_EN)
+
+#define TS_CTL_DST_PORT				TS_319
+#define TS_CTL_DST_PORT_SHIFT			21
+
+#define TS_CTL_MADDR_ALL	\
+	(TS_107 | TS_129 | TS_130 | TS_131 | TS_132)
+
+#define TS_CTL_MADDR_SHIFT			16
+
+/* The PTP event messages - Sync, Delay_Req, Pdelay_Req, and Pdelay_Resp. */
+#define EVENT_MSG_BITS (BIT(0) | BIT(1) | BIT(2) | BIT(3))
+#endif /* CONFIG_TI_CPTS */
+
 struct xgbe_ss_regs {
 	u32	id_ver;
 	u32	synce_count;
@@ -616,6 +673,13 @@ struct gbe_hw_stats {
 #define GBE_MAX_HW_STAT_MODS			9
 #define GBE_HW_STATS_REG_MAP_SZ			0x100
 
+struct ts_ctl {
+	int     uni;
+	u8      dst_port_map;
+	u8      maddr_map;
+	u8      ts_mcast_type;
+};
+
 struct gbe_slave {
 	void __iomem			*port_regs;
 	void __iomem			*emac_regs;
@@ -630,6 +694,7 @@ struct gbe_slave {
 	u32				mac_control;
 	u8				phy_port_t;
 	struct device_node		*phy_node;
+	struct ts_ctl                   ts_ctl;
 	struct list_head		slave_list;
 };
 
@@ -655,6 +720,7 @@ struct gbe_priv {
 	void __iomem			*switch_regs;
 	void __iomem			*host_port_regs;
 	void __iomem			*ale_reg;
+	void __iomem                    *cpts_reg;
 	void __iomem			*sgmii_port_regs;
 	void __iomem			*sgmii_port34_regs;
 	void __iomem			*xgbe_serdes_regs;
@@ -678,6 +744,9 @@ struct gbe_priv {
 	int				num_et_stats;
 	/*  Lock for updating the hwstats */
 	spinlock_t			hw_stats_lock;
+
+	int                             cpts_registered;
+	struct cpts                     *cpts;
 };
 
 struct gbe_intf {
@@ -1904,6 +1973,49 @@ static int keystone_set_settings(struct net_device *ndev,
 	return phy_ethtool_sset(phy, cmd);
 }
 
+#if IS_ENABLED(CONFIG_TI_CPTS)
+static int keystone_get_ts_info(struct net_device *ndev,
+				struct ethtool_ts_info *info)
+{
+	struct netcp_intf *netcp = netdev_priv(ndev);
+	struct gbe_intf *gbe_intf;
+
+	gbe_intf = netcp_module_get_intf_data(&gbe_module, netcp);
+	if (!gbe_intf || !gbe_intf->gbe_dev->cpts)
+		return -EINVAL;
+
+	info->so_timestamping =
+		SOF_TIMESTAMPING_TX_HARDWARE |
+		SOF_TIMESTAMPING_TX_SOFTWARE |
+		SOF_TIMESTAMPING_RX_HARDWARE |
+		SOF_TIMESTAMPING_RX_SOFTWARE |
+		SOF_TIMESTAMPING_SOFTWARE |
+		SOF_TIMESTAMPING_RAW_HARDWARE;
+	info->phc_index = gbe_intf->gbe_dev->cpts->phc_index;
+	info->tx_types =
+		(1 << HWTSTAMP_TX_OFF) |
+		(1 << HWTSTAMP_TX_ON);
+	info->rx_filters =
+		(1 << HWTSTAMP_FILTER_NONE) |
+		(1 << HWTSTAMP_FILTER_PTP_V1_L4_EVENT) |
+		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT);
+	return 0;
+}
+#else
+static int keystone_get_ts_info(struct net_device *ndev,
+				struct ethtool_ts_info *info)
+{
+	info->so_timestamping =
+		SOF_TIMESTAMPING_TX_SOFTWARE |
+		SOF_TIMESTAMPING_RX_SOFTWARE |
+		SOF_TIMESTAMPING_SOFTWARE;
+	info->phc_index = -1;
+	info->tx_types = 0;
+	info->rx_filters = 0;
+	return 0;
+}
+#endif /* CONFIG_TI_CPTS */
+
 static const struct ethtool_ops keystone_ethtool_ops = {
 	.get_drvinfo		= keystone_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
@@ -1914,6 +2026,7 @@ static const struct ethtool_ops keystone_ethtool_ops = {
 	.get_ethtool_stats	= keystone_get_ethtool_stats,
 	.get_settings		= keystone_get_settings,
 	.set_settings		= keystone_set_settings,
+	.get_ts_info		= keystone_get_ts_info,
 };
 
 #define mac_hi(mac)	(((mac)[0] << 0) | ((mac)[1] << 8) |	\
@@ -2357,16 +2470,279 @@ static int gbe_del_vid(void *intf_priv, int vid)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_TI_CPTS)
+#define HAS_PHY_TXTSTAMP(p) ((p)->drv && (p)->drv->txtstamp)
+#define HAS_PHY_RXTSTAMP(p) ((p)->drv && (p)->drv->rxtstamp)
+
+static void gbe_txtstamp(void *context, struct sk_buff *skb)
+{
+	struct gbe_intf *gbe_intf = context;
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+
+	cpts_tx_timestamp(gbe_dev->cpts, skb);
+}
+
+static bool gbe_need_txtstamp(struct gbe_intf *gbe_intf,
+			      const struct netcp_packet *p_info)
+{
+	struct sk_buff *skb = p_info->skb;
+	unsigned int class = ptp_classify_raw(skb);
+
+	if (class == PTP_CLASS_NONE)
+		return false;
+
+	switch (class) {
+	case PTP_CLASS_V1_IPV4:
+	case PTP_CLASS_V1_IPV6:
+	case PTP_CLASS_V2_IPV4:
+	case PTP_CLASS_V2_IPV6:
+	case PTP_CLASS_V2_L2:
+	case (PTP_CLASS_V2_VLAN | PTP_CLASS_L2):
+	case (PTP_CLASS_V2_VLAN | PTP_CLASS_IPV4):
+	case (PTP_CLASS_V2_VLAN | PTP_CLASS_IPV6):
+		return true;
+	}
+
+	return false;
+}
+
+static int gbe_txtstamp_mark_pkt(struct gbe_intf *gbe_intf,
+				 struct netcp_packet *p_info)
+{
+	struct phy_device *phydev = p_info->skb->dev->phydev;
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+
+	if (!(skb_shinfo(p_info->skb)->tx_flags & SKBTX_HW_TSTAMP) ||
+	    !cpts_is_tx_enabled(gbe_dev->cpts))
+		return 0;
+
+	/* If phy has the txtstamp api, assume it will do it.
+	 * We mark it here because skb_tx_timestamp() is called
+	 * after all the txhooks are called.
+	 */
+	if (phydev && HAS_PHY_TXTSTAMP(phydev)) {
+		skb_shinfo(p_info->skb)->tx_flags |= SKBTX_IN_PROGRESS;
+		return 0;
+	}
+
+	if (gbe_need_txtstamp(gbe_intf, p_info)) {
+		p_info->txtstamp = gbe_txtstamp;
+		p_info->ts_context = (void *)gbe_intf;
+		skb_shinfo(p_info->skb)->tx_flags |= SKBTX_IN_PROGRESS;
+	}
+
+	return 0;
+}
+
+static int gbe_rxtstamp(struct gbe_intf *gbe_intf, struct netcp_packet *p_info)
+{
+	struct phy_device *phydev = p_info->skb->dev->phydev;
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+
+	if (p_info->rxtstamp_complete)
+		return 0;
+
+	if (phydev && HAS_PHY_RXTSTAMP(phydev)) {
+		p_info->rxtstamp_complete = true;
+		return 0;
+	}
+
+	cpts_rx_timestamp(gbe_dev->cpts, p_info->skb);
+	p_info->rxtstamp_complete = true;
+
+	return 0;
+}
+
+static int gbe_hwtstamp_get(struct gbe_intf *gbe_intf, struct ifreq *ifr)
+{
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+	struct cpts *cpts = gbe_dev->cpts;
+	struct hwtstamp_config cfg;
+
+	if (!cpts)
+		return -EOPNOTSUPP;
+
+	cfg.flags = 0;
+	cfg.tx_type = cpts_is_tx_enabled(cpts) ?
+		      HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
+	cfg.rx_filter = (cpts_is_rx_enabled(cpts) ?
+			 cpts->rx_enable : HWTSTAMP_FILTER_NONE);
+
+	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+}
+
+static void gbe_hwtstamp(struct gbe_intf *gbe_intf)
+{
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+	struct gbe_slave *slave = gbe_intf->slave;
+	u32 ts_en, seq_id, ctl;
+
+	if (!cpts_is_rx_enabled(gbe_dev->cpts) &&
+	    !cpts_is_tx_enabled(gbe_dev->cpts)) {
+		writel(0, GBE_REG_ADDR(slave, port_regs, ts_ctl));
+		return;
+	}
+
+	seq_id = (30 << TS_SEQ_ID_OFS_SHIFT) | ETH_P_1588;
+	ts_en = EVENT_MSG_BITS << TS_MSG_TYPE_EN_SHIFT;
+	ctl = ETH_P_1588 | TS_TTL_NONZERO |
+		(slave->ts_ctl.dst_port_map << TS_CTL_DST_PORT_SHIFT) |
+		(slave->ts_ctl.uni ?  TS_UNI_EN :
+			slave->ts_ctl.maddr_map << TS_CTL_MADDR_SHIFT);
+
+	if (cpts_is_tx_enabled(gbe_dev->cpts))
+		ts_en |= (TS_TX_ANX_ALL_EN | TS_TX_VLAN_LT1_EN);
+
+	if (cpts_is_rx_enabled(gbe_dev->cpts))
+		ts_en |= (TS_RX_ANX_ALL_EN | TS_RX_VLAN_LT1_EN);
+
+	writel(ts_en,  GBE_REG_ADDR(slave, port_regs, ts_ctl));
+	writel(seq_id, GBE_REG_ADDR(slave, port_regs, ts_seq_ltype));
+	writel(ctl,    GBE_REG_ADDR(slave, port_regs, ts_ctl_ltype2));
+}
+
+static int gbe_hwtstamp_set(struct gbe_intf *gbe_intf, struct ifreq *ifr)
+{
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+	struct cpts *cpts = gbe_dev->cpts;
+	struct hwtstamp_config cfg;
+
+	if (!cpts)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
+		return -EFAULT;
+
+	/* reserved for future extensions */
+	if (cfg.flags)
+		return -EINVAL;
+
+	switch (cfg.tx_type) {
+	case HWTSTAMP_TX_OFF:
+		cpts_tx_enable(cpts, 0);
+		break;
+	case HWTSTAMP_TX_ON:
+		cpts_tx_enable(cpts, 1);
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	switch (cfg.rx_filter) {
+	case HWTSTAMP_FILTER_NONE:
+		cpts_rx_enable(cpts, 0);
+		break;
+	case HWTSTAMP_FILTER_ALL:
+	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
+	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
+		cpts_rx_enable(cpts, HWTSTAMP_FILTER_PTP_V1_L4_EVENT);
+		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
+		break;
+	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
+	case HWTSTAMP_FILTER_PTP_V2_EVENT:
+	case HWTSTAMP_FILTER_PTP_V2_SYNC:
+	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
+		cpts_rx_enable(cpts, HWTSTAMP_FILTER_PTP_V2_EVENT);
+		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		break;
+	default:
+		return -ERANGE;
+	}
+
+	gbe_hwtstamp(gbe_intf);
+
+	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
+}
+
+static void gbe_register_cpts(struct gbe_priv *gbe_dev)
+{
+	if (!gbe_dev->cpts)
+		return;
+
+	if (gbe_dev->cpts_registered > 0)
+		goto done;
+
+	if (cpts_register(gbe_dev->cpts)) {
+		dev_err(gbe_dev->dev, "error registering cpts device\n");
+		return;
+	}
+
+done:
+	++gbe_dev->cpts_registered;
+}
+
+static void gbe_unregister_cpts(struct gbe_priv *gbe_dev)
+{
+	if (!gbe_dev->cpts || (gbe_dev->cpts_registered <= 0))
+		return;
+
+	if (--gbe_dev->cpts_registered)
+		return;
+
+	cpts_unregister(gbe_dev->cpts);
+}
+#else
+static inline int gbe_txtstamp_mark_pkt(struct gbe_intf *gbe_intf,
+					struct netcp_packet *p_info)
+{
+	return 0;
+}
+
+static inline int gbe_rxtstamp(struct gbe_intf *gbe_intf,
+			       struct netcp_packet *p_info)
+{
+	return 0;
+}
+
+static inline int gbe_hwtstamp(struct gbe_intf *gbe_intf,
+			       struct ifreq *ifr, int cmd)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void gbe_register_cpts(struct gbe_priv *gbe_dev)
+{
+}
+
+static inline void gbe_unregister_cpts(struct gbe_priv *gbe_dev)
+{
+}
+
+static inline int gbe_hwtstamp_get(struct gbe_intf *gbe_intf, struct ifreq *req)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int gbe_hwtstamp_set(struct gbe_intf *gbe_intf, struct ifreq *req)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_TI_CPTS */
+
 static int gbe_ioctl(void *intf_priv, struct ifreq *req, int cmd)
 {
 	struct gbe_intf *gbe_intf = intf_priv;
 	struct phy_device *phy = gbe_intf->slave->phy;
-	int ret = -EOPNOTSUPP;
+
+	if (!phy || !phy->drv->hwtstamp) {
+		switch (cmd) {
+		case SIOCGHWTSTAMP:
+			return gbe_hwtstamp_get(gbe_intf, req);
+		case SIOCSHWTSTAMP:
+			return gbe_hwtstamp_set(gbe_intf, req);
+		}
+	}
 
 	if (phy)
-		ret = phy_mii_ioctl(phy, req, cmd);
+		return phy_mii_ioctl(phy, req, cmd);
 
-	return ret;
+	return -EOPNOTSUPP;
 }
 
 static void netcp_ethss_timer(unsigned long arg)
@@ -2402,12 +2778,20 @@ static void netcp_ethss_timer(unsigned long arg)
 	add_timer(&gbe_dev->timer);
 }
 
-static int gbe_tx_hook(int order, void *data, struct netcp_packet *p_info)
+static int gbe_txhook(int order, void *data, struct netcp_packet *p_info)
 {
 	struct gbe_intf *gbe_intf = data;
 
 	p_info->tx_pipe = &gbe_intf->tx_pipe;
-	return 0;
+
+	return gbe_txtstamp_mark_pkt(gbe_intf, p_info);
+}
+
+static int gbe_rxhook(int order, void *data, struct netcp_packet *p_info)
+{
+	struct gbe_intf *gbe_intf = data;
+
+	return gbe_rxtstamp(gbe_intf, p_info);
 }
 
 static int gbe_open(void *intf_priv, struct net_device *ndev)
@@ -2457,11 +2841,14 @@ static int gbe_open(void *intf_priv, struct net_device *ndev)
 	if (ret)
 		goto fail;
 
-	netcp_register_txhook(netcp, GBE_TXHOOK_ORDER, gbe_tx_hook,
-			      gbe_intf);
+	netcp_register_txhook(netcp, GBE_TXHOOK_ORDER, gbe_txhook, gbe_intf);
+	netcp_register_rxhook(netcp, GBE_RXHOOK_ORDER, gbe_rxhook, gbe_intf);
 
 	slave->open = true;
 	netcp_ethss_update_link_state(gbe_dev, slave, ndev);
+
+	gbe_register_cpts(gbe_dev);
+
 	return 0;
 
 fail:
@@ -2473,16 +2860,36 @@ static int gbe_close(void *intf_priv, struct net_device *ndev)
 {
 	struct gbe_intf *gbe_intf = intf_priv;
 	struct netcp_intf *netcp = netdev_priv(ndev);
+	struct gbe_priv *gbe_dev = gbe_intf->gbe_dev;
+
+	gbe_unregister_cpts(gbe_dev);
 
 	gbe_slave_stop(gbe_intf);
-	netcp_unregister_txhook(netcp, GBE_TXHOOK_ORDER, gbe_tx_hook,
-				gbe_intf);
+
+	netcp_unregister_rxhook(netcp, GBE_RXHOOK_ORDER, gbe_rxhook, gbe_intf);
+	netcp_unregister_txhook(netcp, GBE_TXHOOK_ORDER, gbe_txhook, gbe_intf);
 
 	gbe_intf->slave->open = false;
 	atomic_set(&gbe_intf->slave->link_state, NETCP_LINK_STATE_INVALID);
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_TI_CPTS)
+static void init_slave_ts_ctl(struct gbe_slave *slave)
+{
+	slave->ts_ctl.uni = 1;
+	slave->ts_ctl.dst_port_map =
+		(TS_CTL_DST_PORT >> TS_CTL_DST_PORT_SHIFT) & 0x3;
+	slave->ts_ctl.maddr_map =
+		(TS_CTL_MADDR_ALL >> TS_CTL_MADDR_SHIFT) & 0x1f;
+}
+
+#else
+static void init_slave_ts_ctl(struct gbe_slave *slave)
+{
+}
+#endif /* CONFIG_TI_CPTS */
+
 static int init_slave(struct gbe_priv *gbe_dev, struct gbe_slave *slave,
 		      struct device_node *node)
 {
@@ -2597,6 +3004,8 @@ static int init_slave(struct gbe_priv *gbe_dev, struct gbe_slave *slave,
 	}
 
 	atomic_set(&slave->link_state, NETCP_LINK_STATE_INVALID);
+
+	init_slave_ts_ctl(slave);
 	return 0;
 }
 
@@ -2787,6 +3196,7 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
 			XGBE10_HW_STATS_OFFSET + (GBE_HW_STATS_REG_MAP_SZ * i);
 
 	gbe_dev->ale_reg = gbe_dev->switch_regs + XGBE10_ALE_OFFSET;
+	gbe_dev->cpts_reg = gbe_dev->switch_regs + XGBE10_CPTS_OFFSET;
 	gbe_dev->ale_ports = gbe_dev->max_num_ports;
 	gbe_dev->host_port = XGBE10_HOST_PORT_NUM;
 	gbe_dev->ale_entries = XGBE10_NUM_ALE_ENTRIES;
@@ -2909,6 +3319,7 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
 			(GBE_HW_STATS_REG_MAP_SZ * (i & 0x1));
 	}
 
+	gbe_dev->cpts_reg = gbe_dev->switch_regs + GBE13_CPTS_OFFSET;
 	gbe_dev->ale_reg = gbe_dev->switch_regs + GBE13_ALE_OFFSET;
 	gbe_dev->ale_ports = gbe_dev->max_num_ports;
 	gbe_dev->host_port = GBE13_HOST_PORT_NUM;
@@ -2998,6 +3409,7 @@ static int set_gbenu_ethss_priv(struct gbe_priv *gbe_dev,
 		gbe_dev->hw_stats_regs[i] = gbe_dev->switch_regs +
 			GBENU_HW_STATS_OFFSET + (GBENU_HW_STATS_REG_MAP_SZ * i);
 
+	gbe_dev->cpts_reg = gbe_dev->switch_regs + GBENU_CPTS_OFFSET;
 	gbe_dev->ale_reg = gbe_dev->switch_regs + GBENU_ALE_OFFSET;
 	gbe_dev->ale_ports = gbe_dev->max_num_ports;
 	gbe_dev->host_port = GBENU_HOST_PORT_NUM;
@@ -3179,6 +3591,12 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
 		dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
 	}
 
+	gbe_dev->cpts = cpts_create(gbe_dev->dev, gbe_dev->cpts_reg, node);
+	if (IS_ENABLED(CONFIG_TI_CPTS) && IS_ERR(gbe_dev->cpts)) {
+		ret = PTR_ERR(gbe_dev->cpts);
+		goto free_sec_ports;
+	}
+
 	/* initialize host port */
 	gbe_init_host_port(gbe_dev);
 
@@ -3267,6 +3685,7 @@ static int gbe_remove(struct netcp_device *netcp_device, void *inst_priv)
 	struct gbe_priv *gbe_dev = inst_priv;
 
 	del_timer_sync(&gbe_dev->timer);
+	cpts_release(gbe_dev->cpts);
 	cpsw_ale_stop(gbe_dev->ale);
 	cpsw_ale_destroy(gbe_dev->ale);
 	netcp_txpipe_close(&gbe_dev->tx_pipe);
-- 
2.10.1

^ permalink raw reply related

* [PATCH 0/6] net: ethernet: ti: cpts: update and enable support on keystone 2 socs
From: Grygorii Strashko @ 2016-11-28 23:04 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko

Time Synchronization (CPTS) submodule which is present on KeyStone 66AK2HK/E/L/Gx
1G Switch Subsystem provides the same basic functionality as OMAP CPSW CPTS, but
with few additional features:
- CPTS rftclk selection (reg CPTS_RFTCLK_SEL). This feature is declared
  to be supported on am437x SoCs also.
- CPTS HW_TS_PUSH events which can be generated by external low frequency
  time stamp channels (66AK2E/L/Gx, am437x)
- one Time Stamp Compare (TS_COMP) output which is reused for PTP PPS feature
  implementation (66AK2E/L/Gx).
Hence, This series enables basic CPTS support on Keystone 2 SoCs by resuing
current CPSW CPTS driver.

Links on docs:
 66AK2H/kx http://www.ti.com/lit/pdf/sprugv9
 66AK2E/Lx http://www.ti.com/lit/pdf/spruhz3
 66AK2Gx http://www.ti.com/lit/pdf/spruhy8

Note. This series based on top of  preparation series
      "[PATCH v2 00/13] net: ethernet: ti: cpts: update and fixes"

Tested on am437x-idk, am57xx-evm, 66AK2HK, 66AK2E, 66AK2G
Tests:
   server: ptp4l -E -2 -H -i eth0  -l 6 -m -q -p /dev/ptp0
   client:  ptp4l -E -2 -H -i eth0  -l 6 -m -q -p /dev/ptp0 -s

   testptp -g && sleep X && testptp -g

   testptp -c
   testptp -g
   testptp -s
   testptp -k 25
   testptp -e 3

   testptp -P 1 && .ppstest /dev/pps0

Grygorii Strashko (4):
  net: ethernet: ti: cpts: add support for ext rftclk selection
  net: ethernet: ti: cpts: add support of cpts HW_TS_PUSH
  net: ethernet: ti: cpts: add ptp pps support
  ARM: dts: keystone: enable time synchronization (cpts) submodule

Murali Karicheri (1):
  ARM: keystone: dts: fix netcp clocks and add names

WingMan Kwok (1):
  net: ethernet: ti: netcp: add support of cpts

 Documentation/devicetree/bindings/net/cpsw.txt     |   4 +
 .../devicetree/bindings/net/keystone-netcp.txt     |  25 ++
 arch/arm/boot/dts/keystone-k2e-netcp.dtsi          |   6 +-
 arch/arm/boot/dts/keystone-k2hk-netcp.dtsi         |   4 +-
 arch/arm/boot/dts/keystone-k2l-netcp.dtsi          |   6 +-
 drivers/net/ethernet/ti/Kconfig                    |   7 +-
 drivers/net/ethernet/ti/cpts.c                     | 343 +++++++++++++++-
 drivers/net/ethernet/ti/cpts.h                     |  28 +-
 drivers/net/ethernet/ti/netcp.h                    |   2 +-
 drivers/net/ethernet/ti/netcp_core.c               |  18 +-
 drivers/net/ethernet/ti/netcp_ethss.c              | 437 ++++++++++++++++++++-
 11 files changed, 853 insertions(+), 27 deletions(-)

-- 
2.10.1

^ permalink raw reply

* [PATCH  v2 13/13] net: ethernet: ti: cpts: fix overflow check period
From: Grygorii Strashko @ 2016-11-28 23:03 UTC (permalink / raw)
  To: David S. Miller, netdev, Mugunthan V N, Richard Cochran
  Cc: Sekhar Nori, linux-kernel, linux-omap, Rob Herring, devicetree,
	Murali Karicheri, Wingman Kwok, Grygorii Strashko, John Stultz,
	Thomas Gleixner
In-Reply-To: <20161128230337.6731-1-grygorii.strashko@ti.com>

The CPTS drivers uses 8sec period for overflow checking with
assumption that CPTS retclk will not exceed 500MHz. But that's not
true on some TI platforms (Kesytone 2). As result, it is possible that
CPTS counter will overflow more than once between two readings.

Hence, fix it by selecting overflow check period dynamically as
max_sec_before_overflow/2, where
 max_sec_before_overflow = max_counter_val / rftclk_freq.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpts.c | 10 +++++++---
 drivers/net/ethernet/ti/cpts.h |  4 +---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 4761d8c..c96a94a 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -245,7 +245,7 @@ static void cpts_overflow_check(struct work_struct *work)
 
 	cpts_ptp_gettime(&cpts->info, &ts);
 	pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
-	schedule_delayed_work(&cpts->overflow_work, CPTS_OVERFLOW_PERIOD);
+	schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
 }
 
 static int cpts_match(struct sk_buff *skb, unsigned int ptp_class,
@@ -379,8 +379,7 @@ int cpts_register(struct cpts *cpts)
 	}
 	cpts->phc_index = ptp_clock_index(cpts->clock);
 
-	schedule_delayed_work(&cpts->overflow_work, CPTS_OVERFLOW_PERIOD);
-
+	schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
 	return 0;
 
 err_ptp:
@@ -421,6 +420,11 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
 	if (maxsec > 600 && cpts->cc.mask > UINT_MAX)
 		maxsec = 600;
 
+	/* Calc overflow check period (maxsec / 2) */
+	cpts->ov_check_period = (HZ * maxsec) / 2;
+	dev_info(cpts->dev, "cpts: overflow check period %lu (jiffies)\n",
+		 cpts->ov_check_period);
+
 	if (cpts->cc_mult || cpts->cc.shift)
 		return;
 
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index 5da23af..c96eca2 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -97,9 +97,6 @@ enum {
 	CPTS_EV_TX,   /* Ethernet Transmit Event */
 };
 
-/* This covers any input clock up to about 500 MHz. */
-#define CPTS_OVERFLOW_PERIOD (HZ * 8)
-
 #define CPTS_FIFO_DEPTH 16
 #define CPTS_MAX_EVENTS 32
 
@@ -127,6 +124,7 @@ struct cpts {
 	struct list_head events;
 	struct list_head pool;
 	struct cpts_event pool_data[CPTS_MAX_EVENTS];
+	unsigned long ov_check_period;
 };
 
 void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
-- 
2.10.1

^ permalink raw reply related


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