Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] dt-bindings: Add "vot" vendor prefix
From: Rob Herring @ 2017-12-26 21:49 UTC (permalink / raw)
  To: David Lechner
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Noralf Trønnes,
	limor-6aDhHjTmHzzR7s880joybQ, Linus Walleij, Mark Rutland,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1513881187-3197-2-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>

On Thu, Dec 21, 2017 at 12:33:05PM -0600, David Lechner wrote:
> This adds a vendor prefix "vot" for Vision Optical Technology Co., Ltd.
> They make LCD displays.
> 
> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@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 1/2] dt-bindings: Document mti,mips-cpc binding
From: Rob Herring @ 2017-12-26 21:48 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, Paul Burton,
	Aleksandar Markovic, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Douglas Leung, Goran Ferenc, James Hogan,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, Miodrag Dinic,
	Petar Jovanovic, Raghu Gandham
In-Reply-To: <1513869627-15391-2-git-send-email-aleksandar.markovic-FblTVreYubkAvxtiuMwx3w@public.gmane.org>

On Thu, Dec 21, 2017 at 04:20:23PM +0100, Aleksandar Markovic wrote:
> From: Paul Burton <paul.burton-8NJIiSa5LzA@public.gmane.org>
> 
> Document a binding for the MIPS Cluster Power Controller (CPC) which
> simply allows the device tree to specify where the CPC registers are
> located.
> 
> Signed-off-by: Paul Burton <paul.burton-8NJIiSa5LzA@public.gmane.org>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic-8NJIiSa5LzA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/misc/mti,mips-cpc.txt | 8 ++++++++

power controllers are usually documented under bindings/power/

>  1 file changed, 8 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/mti,mips-cpc.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/mti,mips-cpc.txt b/Documentation/devicetree/bindings/misc/mti,mips-cpc.txt
> new file mode 100644
> index 0000000..c6b8251
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mti,mips-cpc.txt
> @@ -0,0 +1,8 @@
> +Binding for MIPS Cluster Power Controller (CPC).
> +
> +This binding allows a system to specify where the CPC registers are
> +located.
> +
> +Required properties:
> +compatible : Should be "mti,mips-cpc".
> +regs: Should describe the address & size of the CPC register region.
> -- 
> 2.7.4
> 
--
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 1/5] dt-bindings: at24: consistently document the compatible property
From: Rob Herring @ 2017-12-26 21:32 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Javier Martinez Canillas, Bartosz Golaszewski, Andy Shevchenko,
	Mark Rutland, David Lechner, Divagar Mohandass, Linux I2C,
	devicetree, Linux Kernel
In-Reply-To: <5adf70b0-51a2-f1b1-a5e9-2b9c4ef2ff1e@axentia.se>

On Fri, Dec 22, 2017 at 05:58:29PM +0100, Peter Rosin wrote:
> On 2017-12-22 00:38, Javier Martinez Canillas wrote:
> > Hello Peter,
> > 
> > On Fri, Dec 22, 2017 at 12:07 AM, Peter Rosin <peda@axentia.se> wrote:
> >> On 2017-12-21 21:27, Javier Martinez Canillas wrote:
> >>> Hello Peter,
> >>>
> >>> On Thu, Dec 21, 2017 at 5:20 PM, Peter Rosin <peda@axentia.se> wrote:
> >>>> On 2017-12-21 14:48, Bartosz Golaszewski wrote:
> >>>>> Current description of the compatible property for at24 is quite vague.
> >>>>>
> >>>>> Specify an exact list of accepted compatibles and document the - now
> >>>>> deprecated - strings which were previously used in device tree files.
> >>>>
> >>>> Why is it suddenly deprecated to correctly specify what hardware you
> >>>> have, e.g. "nxp,24c32". In this case the manufacturer is nxp, damnit.
> >>>
> >>> Sorry but I disagree.
> >>>
> >>> When you specify a compatible string, you are not specifying a
> >>> particular hardware but a device programming model.
> >>
> >> That's not what it says in https://elinux.org/Device_Tree_Usage
> > 
> > I think the most up-to-date DT reference is at:
> > 
> > https://www.devicetree.org/
> > 
> >> in the "Understanding the compatible Property" section. I quote:
> >>
> >>         compatible is a list of strings. The first string in the
> >>         list specifies the exact device that the node represents
> >>         in the form "<manufacturer>,<model>". The following strings
> >>         represent other devices that the device is compatible with.
> >>
> >> Pretty clearly talks about devices and not programming models. But
> >> maybe I shouldn't trust that reference? What should I be reading
> >> instead?
> >>
> > 
> > For example the latest spec draft
> > (https://github.com/devicetree-org/devicetree-specification/releases/download/v0.2-rc1/devicetree-specification-v0.2-rc1.pdf)
> > says:
> > 
> > "The compatible property value consists of one or more strings that
> > define the specific programming model for the device. This list of
> > strings should be used by a client program for device driver
> > selection. The property value consists of a concatenated list of null
> > terminated strings, from most specific to most general. They allow a
> > device to express its compatibility with a family of similar devices,
> > potentially allowing a single device driver to match against several
> > devices."
> > 
> > The recommended format is "manufacturer,model", where manufacturer is
> > a string describing the name of the manufacturer (such as a stock
> > ticker symbol), and model specifies the model number."
> > 
> > Example:
> > 
> > compatible = "fsl,mpc8641", "ns16550";
> > 
> > In this example, an operating system would first try to locate a
> > device driver that supported fsl,mpc8641. If a driver was not found,
> > it would then try to locate a driver that supported the more general
> > ns16550 device type."
> 
> Yes, that's a bit different from the wording on the elinux site. Thanks
> for the pointer! Google was not my friend on this occasion, since I
> managed to miss that site in my searches. Maybe it has gotten a higher
> rank now that 0.2 is out, because now I find it easily?

The wording may be different, but both (and Peter) are correct.

And ns16550 is a perfect example of why that string alone is not 
sufficient. Just go count the number of variants and quirks in the 
kernel 8250 driver.

> >>> It's very common to use a compatible string that doesn't match exactly
> >>> the specific hardware used. That's why it's called _compatible_ BTW.
> >>
> >> That's not how I read the above.
> >>
> > 
> > That's not how I read it nor my experience with DT, but of course I
> > may be wrong on this.
> 
> Either way, it should not be wrong to specify the more specific binding
> before the generic fallback, as is done in the at91-tse850-3.dts
> example I gave below.

Indeed. I don't think I've ever said anyone is being too specific.

> >>> For example when a DTS define a UART node with an ns16550 compatible,
> >>> they don't really mean that have a UART IC manufactured by National
> >>> Semiconductor.
> >>
> >> That just tells me that most people are a little bit lazy and ready
> >> to cut a corner or two when they can get away with it. Or that there
> >> is some form of misunderstanding at work...
> >>
> > 
> > For example, I usually see that different SoC families from the same
> > vendor use the same compatible string for integrated peripherals just
> > because are the same from a programming model point of view.
> > 
> > TI am33xx SoCs use a lot of omap3 compatible strings on their nodes
> > and its similar on Exynos SoCs which are the two ARM SoCs I'm most
> > familiar with. Following your logic that's wrong and instead a new
> > compatible string should be added for the GPIO or pinctrl drivers even
> > when are the same because refer to different devices.
> 
> Considering the above quote from the actual DT spec, I wouldn't say wrong.
> But I'd still say that it should be preferred to list the actual device
> before the fallback to some generic programming model compatible or some
> previous version of the hardware/IP-block. Just in case an unintended
> difference is discovered late in the game...

Let me clear, "generic" compatibles alone are wrong.

> 
> >>>> Sure, it happens to be compatible with "atmel,24c32", but that is
> >>>> supposed to be written with a fallback as
> >>>>
> >>>>         "nxp,24c32", "atmel,24c32"
> >>>
> >>> This isn't a requirement really, systems integrators are free to use
> >>> more than one <manufacturer,model> tuple in the compatible string if
> >>> they want the DTB to be future proof, just in case there's a need for
> >>> a more specific driver or if the programming model happened to not be
> >>> the same at the end. This is usually done for the boards compatible
> >>> string as an example, even when there isn't a struct machine_desc for
> >>> the specific board compatible and only for the SoC family.
> >>>
> >>> So it's OK if you want to define the compatible as "nxp,24c32",
> >>> "atmel,24c32", but that's a general OF concept and not something
> >>> related to the at24 eeprom driver so I'm not sure if it should be
> >>> mentioned in the at24 DT binding doc.
> >>
> >> One problem is that if "nxp,24c32" (or "nxp,24c02" as in the example
> >> below) is not a valid compatible, the tooling will be correct to
> >> complain about it. Currently, it is just a checkpatch deficiency that
> >> it complains like this:
> >>
> >> $ scripts/checkpatch.pl -f arch/arm/boot/dts/at91-tse850-3.dts
> >> WARNING: DT compatible string "nxp,24c02" appears un-documented -- check ./Documentation/devicetree/bindings/
> >> #249: FILE: arch/arm/boot/dts/at91-tse850-3.dts:249:
> >> +               compatible = "nxp,24c02", "atmel,24c02";
> >>
> > 
> > But isn't that a bug in checkpatch? as long as there's a valid
> > <manufacturer,model> tuple in the compatible string, it shouldn't
> > complain.
> 
> Then there is a significant risk that speeling mistakes are not
> caught. So, I don't think it will be considered a bug unless a DT
> guru says so of course. I think the checkpatch intention is to catch
> all undocumented compatibles. But I don't know of course...

checkpatch.pl is just plain stupid. Who wrote that check anyway? Because 
bindings are not machine parseable, compatible string checks are pretty 
much just a grep for the string in all binding docs. So "The" or 
"compatible" would be valid compatible strings for checkpatch.pl.

If you want to quiet checkpatch.pl, I'd suggest documenting compatible 
strings as "<vendor>,foo" and having checkpatch match any string for 
<vendor>. We already handle vendor,<soc>-block type compatible 
descriptions.

Rob

^ permalink raw reply

* Re: [RESEND][PATCH 1/4] of: platform: populate /firmware/ node from of_platform_default_populate_init()
From: Bjorn Andersson @ 2017-12-26 21:26 UTC (permalink / raw)
  To: Sudeep Holla, Andy Gross
  Cc: LAKML, lkml, devicetree, Rob Herring, Arnd Bergmann, Rob Herring
In-Reply-To: <1506595562-10592-2-git-send-email-sudeep.holla-5wv7dgnIgG8@public.gmane.org>

On Thu, Sep 28, 2017 at 3:45 AM, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> wrote:
> Since "/firmware" does not have its own "compatible" property as it's
> just collection of nodes representing firmware interface, it's sub-nodes
> are not populated during system initialization.
>
> Currently different firmware drivers search the /firmware/ node and
> populate the sub-node devices selectively. Instead we can populate
> the /firmware/ node during init to avoid more drivers continuing to
> populate the devices selectively.
>
> To generalize the solution this patch populates the /firmware/ node
> explicitly from of_platform_default_populate_init().
>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> ---

Andy, as you picked patch 2/4 into your v4.16 -next branch we no
longer probe the qcom_scm device, causing various breakage. Can you
please pick this patch as well (it has Rob's ack).

Regards,
Bjorn

>  drivers/of/platform.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index ac15d0e3d27d..3a213a6aee89 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -515,6 +515,10 @@ static int __init of_platform_default_populate_init(void)
>                         of_platform_device_create(node, NULL, NULL);
>         }
>
> +       node = of_find_node_by_path("/firmware");
> +       if (node)
> +               of_platform_populate(node, NULL, NULL, NULL);
> +
>         /* Populate everything else. */
>         of_platform_default_populate(NULL, NULL, NULL);
>
> --
> 2.7.4
>
--
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: [RFC 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433
From: Rob Herring @ 2017-12-26 21:11 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, krzk-DgEjT+Ai2ygdnm+yROfE0A,
	jingoohan1-Re5JQEeQqe8AvxtiuMwx3w, kgene-DgEjT+Ai2ygdnm+yROfE0A,
	lorenzo.pieralisi-5wv7dgnIgG8
In-Reply-To: <20171221121408.22636-2-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Thu, Dec 21, 2017 at 09:14:07PM +0900, Jaehoon Chung wrote:
> Exynos5433 has the PCIe for WiFi.
> Added the codes relevant to PCIe for supporting the exynos5433.
> Also changed the binding documentation name to
> 'samsung,exynos-pcie.txt'.
> (It's not only exynos5440 anymore.)
> 
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
>  drivers/pci/dwc/pci-exynos.c                       | 183 ++++++++++++++++-----
>  2 files changed, 144 insertions(+), 41 deletions(-)
>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt => samsung,exynos-pcie.txt} (97%)
> 
> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> similarity index 97%
> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> index 34a11bfbfb60..958dcc150505 100644
> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "samsung,exynos5440-pcie"
> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"

Quite a lot of driver changes for just a new compatible.

>  - reg: base addresses and lengths of the PCIe controller,

For example, you're adding the DBI registers which is not documented 
here. 

Perhaps it is time to remove the old phy support before adding a new 
platform.

>  	the PHY controller, additional register for the PHY controller.
>  	(Registers for the PHY controller are DEPRECATED.
> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> index 5596fdedbb94..8dee2e90347e 100644
> --- a/drivers/pci/dwc/pci-exynos.c
> +++ b/drivers/pci/dwc/pci-exynos.c
> @@ -40,6 +40,8 @@
>  #define PCIE_IRQ_SPECIAL		0x008
>  #define PCIE_IRQ_EN_PULSE		0x00c
>  #define PCIE_IRQ_EN_LEVEL		0x010
> +#define PCIE_SW_WAKE			0x018
> +#define PCIE_BUS_EN			BIT(1)
>  #define IRQ_MSI_ENABLE			BIT(2)
>  #define PCIE_IRQ_EN_SPECIAL		0x014
>  #define PCIE_PWR_RESET			0x018
> @@ -49,7 +51,8 @@
>  #define PCIE_NONSTICKY_RESET		0x024
>  #define PCIE_APP_INIT_RESET		0x028
>  #define PCIE_APP_LTSSM_ENABLE		0x02c
> -#define PCIE_ELBI_RDLH_LINKUP		0x064
> +#define PCIE_ELBI_RDLH_LINKUP		0x074
> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
>  #define PCIE_ELBI_SLV_AWMISC		0x11c
>  #define PCIE_ELBI_SLV_ARMISC		0x120
> @@ -94,6 +97,10 @@
>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>  #define PCIE_PHY_TRSV3_LVCC		0x31c
>  
> +/* DBI register */
> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
> +#define DBI_RO_WR_EN			BIT(0)
> +
>  struct exynos_pcie_mem_res {
>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops exynos5440_pcie_ops = {
>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
>  };
>  
> +static int exynos5433_pcie_get_mem_resources(struct platform_device *pdev,
> +					     struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +	struct resource *res;
> +
> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
> +	if (!ep->mem_res)
> +		return -ENOMEM;
> +
> +	/* External Local Bus interface(ELBI) Register */

These are standard DW registers IIRC. So the DW core should handle this.

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ep->mem_res->elbi_base))
> +		return PTR_ERR(ep->mem_res->elbi_base);
> +
> +	/* Data Bus Interface(DBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);

This is handled by the DW plat driver. Perhaps the DW core should handle 
it too. 

Does the 5440 really not have DBI registers or you just happen to not 
need to access them?

> +	if (IS_ERR(pci->dbi_base))
> +		return PTR_ERR(pci->dbi_base);
> +
> +	return 0;
> +}
> +
> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +
> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
> +	if (!ep->clk_res)
> +		return -ENOMEM;
> +
> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
> +	if (IS_ERR(ep->clk_res->clk)) {
> +		dev_err(dev, "Failed to get pcie rc clock\n");
> +		return PTR_ERR(ep->clk_res->clk);
> +	}
> +
> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
> +	if (IS_ERR(ep->clk_res->bus_clk)) {
> +		dev_err(dev, "Failed to get pcie bus clock\n");
> +		return PTR_ERR(ep->clk_res->bus_clk);
> +	}
> +
> +	return 0;
> +}

Can't you reuse exynos5440_pcie_get_clk_resources? They appear to be the 
same.

> +
> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
> +{
> +	clk_disable_unprepare(ep->clk_res->bus_clk);
> +	clk_disable_unprepare(ep->clk_res->clk);
> +}
> +
> +
> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
> +{
> +	struct dw_pcie *pci = ep->pci;
> +	struct device *dev = pci->dev;
> +	int ret;
> +
> +	ret = clk_prepare_enable(ep->clk_res->clk);
> +	if (ret) {
> +		dev_err(dev, "cannot enable pcie rc clock");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot enable pcie bus clock");
> +		goto err_bus_clk;
> +	}
> +
> +	return 0;
> +
> +err_bus_clk:
> +	clk_disable_unprepare(ep->clk_res->clk);
> +
> +	return ret;
> +}

Ditto.

Rob
--
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 V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values
From: Rob Herring @ 2017-12-26 20:29 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Rafael Wysocki,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <476d7ae69184d787ccc6d99f8df6069007fd0a91.1513591822.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote:
> On some platforms the exact frequency or voltage may be hidden from the
> OS by the firmware. Allow such configurations to pass magic values in
> the "opp-hz" or the "opp-microvolt" properties, which should be
> interpreted in a platform dependent way.
> 
> Reviewed-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 4e4f30288c8b..00a3bdbd0f1f 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -167,6 +167,12 @@ properties.
>    functioning of the current device at the current OPP (where this property is
>    present).
>  
> +
> +On some platforms the exact frequency or voltage may be hidden from the OS by
> +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain
> +magic values that represent the frequency or voltage in a firmware dependent
> +way, for example an index of an array in the firmware.

I'm still not convinced this is a good idea. If you have firmware 
partially managing things, then I think we should have platform specific 
bindings or drivers. 

This is complex enough I'm not taking silence from Stephen as an okay.

Rob
--
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: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values
From: Rob Herring @ 2017-12-26 20:23 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Stephen Boyd, Ulf Hansson, Kevin Hilman, Viresh Kumar,
	Nishanth Menon, Rafael Wysocki,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Vincent Guittot,
	Rajendra Nayak, Sudeep Holla,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20171130065907.GI11413@vireshk-i7>

On Thu, Nov 30, 2017 at 12:59 AM, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 29-11-17, 16:50, Stephen Boyd wrote:
>> Sorry it still makes zero sense to me. It seems that we're trying
>> to make the OPP table parsing generic just for the sake of code
>> brevity.
>
> Not just the code but bindings as well to make sure we don't add a new
> property (similar to earlier ones) for every platform that wants to
> use performance states.
>
>> Is this the goal? From a DT writer perspective it seems
>> confusing to say that opp-microvolt is sometimes a microvolt and
>> sometimes not a microvolt.
>
> Well it would still represent the voltage but not in microvolt units
> as the platform guys decided to hide those values from kernel and
> handle them directly in firmware.
>
>> Why can't the SoC specific genpd
>> driver parse something like "qcom,corner" instead out of the
>> node?
>
> Sure we can, but that means that a new property will be required for
> the next platform.
>
> I did it this way as Kevin (and Rob) suggested NOT to add another
> property but use the earlier ones as we aren't passing anything new
> here, just that the units of the property are different. For another
> SoC, we may want to hide both freq and voltage values from kernel and
> pass firmware dependent values. Should we add two new properties for
> that SoC then ?
>
>> BTW, I don't believe I have a use-case where I want to express
>> power domain OPP tables.
>
> I do remember that you once said [1] that you may want to pass the
> real voltage values as well via DT. And so I thought that you can pass
> performance-state (corner) in opp-hz and real voltage values in
> opp-microvolt.
>
>> I have many devices that all have
>> different frequencies that are all tied into the same power
>> domain. This binding makes it look like we can only have one
>> frequency per domain which won't work.
>
> No, that isn't the case. Looks like we have some confusion here. Let
> me try with a simple example:
>
>         foo: foo-power-domain@09000000 {
>                 compatible = "foo,genpd";
>                 #power-domain-cells = <0>;
>                 operating-points-v2 = <&domain_opp_table>;
>         };
>
>         cpu0: cpu@0 {
>                 compatible = "arm,cortex-a53", "arm,armv8";
>                 ...
>                 operating-points-v2 = <&cpu_opp_table>;
>                 power-domains = <&foo>;
>         };
>
>
>         domain_opp_table: domain_opp_table {
>                 compatible = "operating-points-v2";
>
>                 domain_opp_1: opp00 {
>                         opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */
>                 };
>                 domain_opp_2: opp01 {
>                         opp-hz = /bits/ 64 <2>;
>                 };
>                 domain_opp_3: opp02 {
>                         opp-hz = /bits/ 64 <3>;
>                 };
>         };
>
>         cpu_opp_table: cpu_opp_table {
>                 compatible = "operating-points-v2";
>                 opp-shared;
>
>                 opp00 {
>                         opp-hz = /bits/ 64 <208000000>;
>                         clock-latency-ns = <500000>;
>                         power-domain-opp = <&domain_opp_1>;

What is this? opp00 here is not a device. One OPP should not point to
another. "power-domain-opp" is only supposed to appear in devices
alongside power-domains properties.

Rob
--
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 1/3] mtd: spi-nor: add optional DMA-safe bounce buffer for data transfer
From: Trent Piepho @ 2017-12-26 19:43 UTC (permalink / raw)
  To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
	broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	cyrille.pitchen-yU5RGvR974pGWvitb5QawA@public.gmane.org,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
	computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
	vigneshr-l0cyMroinI0@public.gmane.org,
	richard-/L3Ra7n9ekc@public.gmane.org,
	marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org,
	robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	radu.pirea-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <fc2440c6f52877f28286d89691049e5cba10e6a7.1514087323.git.cyrille.pitchen-yU5RGvR974pGWvitb5QawA@public.gmane.org>

On Sun, 2017-12-24 at 05:36 +0100, Cyrille Pitchen wrote:
> 
> Then the patch adds two hardware capabilities for SPI flash controllers,
> SNOR_HWCAPS_WR_BOUNCE and SNOR_HWCAPS_RD_BOUNCE.

Are there any drivers for which a bounce buffer is NOT needed when the
tx/rx buffer is not in DMA safe memory?  Maybe it would make more sense
to invert the sense of these flags, so that they indicate the driver
does not need DMA safe buffers, if that is the uncommon/non-existent
case, so that fewer drivers need to be modified to to be fixed?

> +static bool spi_nor_is_dma_safe(const void *buf)
> +{
> +	if (is_vmalloc_addr(buf))
> +		return false;
> +
> +#ifdef CONFIG_HIGHMEM
> +	if ((unsigned long)buf >= PKMAP_BASE &&
> +	    (unsigned long)buf < (PKMAP_BASE + (LAST_PKMAP * PAGE_SIZE)))
> +		return false;
> +#endif

It looks like:

(unsigned long)addr >= PKMAP_ADDR(0) &&
(unsigned long)addr < PKMAP_ADDR(LAST_PKMAP)

is the expression used in the highmem code.  But really, isn't this
begging for is_highmem_addr() in include/linux/mm.h that can always
return false when highmem is not enabled?

In order to be safe, this must be called when nor->lock is held, right?
 Otherwise it could race against two callers allocating the buffer at
the same time.  That should probably be noted in the kerneldoc comments
for this function, which should also be written.

> +static int spi_nor_get_bounce_buffer(struct spi_nor *nor,
> +				     u_char **buffer,
> +				     size_t *buffer_size)
> +{

> +
> +	*buffer = nor->bounce_buffer;
> +	*buffer_size = size;

So the buffer is returned via the parameter, and also via a field
inside nor.  Seems redundant.  Consider address could be returned via
the function return value coupled with PTR_ERR() for the error cases. 
Or not return address at all since it's available via nor-
>bounce_buffer.

>  {
>  	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> +	bool use_bounce = (nor->flags & SNOR_F_USE_RD_BOUNCE) &&
> +			  !spi_nor_is_dma_safe(buf);
> +	u_char *buffer = buf;
> +	size_t buffer_size = 0;
>  	int ret;
>  
>  	dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
> @@ -1268,13 +1324,23 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>  	if (ret)
>  		return ret;
>  
> +	if (use_bounce) {
> +		ret = spi_nor_get_bounce_buffer(nor, &buffer, &buffer_size);
> +		if (ret < 0)
> +			goto read_err;
> +	}

This pattern, check if bounce is enabled, check if address is dma-
unsafe, get bounce buffer, seems to be very common.  Could it be
refactored into one helper?

u_char *buffer = spi_nor_check_bounce(nor, buf, len, &buffer_size);
if (IS_ERR(buffer)) 
    return PTR_ERR(buffer);
// buffer = nor->bounce_buffer or buf, whichever is correct
// buffer_size = len or bounce buffer size, whichever is correct

Could spi_nor_read_sfdp_dma_unsafe() also use this buffer?

^ permalink raw reply

* Re: [PATCHv3 1/3] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Florian Fainelli @ 2017-12-26 19:34 UTC (permalink / raw)
  To: Andrew Lunn, jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	arnd.bergmann-QSEj5FYQhm4dnm+yROfE0A,
	ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A, Jassi Brar
In-Reply-To: <20171221135541.GC15416-g2DYL2Zd6BY@public.gmane.org>

On 12/21/2017 05:55 AM, Andrew Lunn wrote:
>> +- mdio device tree subnode: When the Netsec has a phy connected to its local
>> +		mdio, there must be device tree subnode with the following
>> +		required properties:
>> +
>> +	- compatible: Must be "socionext,snq-mdio".
> 
> Is there a need for a compatible string? Is there different versions
> of the MDIO bus hardware? If it was an independent MDIO bus driver,
> then yes, you need a compatible string. But since it is embedded in
> the MAC driver, there should not be a need.

I don't see a problem with doing that though, it may be extra
information, but if we ever have a standalone MDIO bus controller that
happens to be supported by the same HW/driver, and the Ethernet driver
delegates the MDIO management to this MDIO bus driver, then having the
compatible string is kind of mandatory for proper matching/identification.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 3/6] media: dt-bindings: Add bindings for TDA1997X
From: Rob Herring @ 2017-12-26 19:03 UTC (permalink / raw)
  To: Tim Harvey
  Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, Steve Longerbeam, Philipp Zabel,
	Hans Verkuil, Mauro Carvalho Chehab
In-Reply-To: <1513447230-30948-4-git-send-email-tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>

On Sat, Dec 16, 2017 at 10:00:27AM -0800, Tim Harvey wrote:
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
> ---
> v5:
>  - added Sakari's ack
> 
> v4:
>  - move include/dt-bindings/media/tda1997x.h to bindings patch
>  - clarify port node details
> 
> v3:
>  - fix typo
> 
> v2:
>  - add vendor prefix and remove _ from vidout-portcfg
>  - remove _ from labels
>  - remove max-pixel-rate property
>  - describe and provide example for single output port
>  - update to new audio port bindings
> ---
>  .../devicetree/bindings/media/i2c/tda1997x.txt     | 179 +++++++++++++++++++++
>  include/dt-bindings/media/tda1997x.h               |  78 +++++++++
>  2 files changed, 257 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/tda1997x.txt
>  create mode 100644 include/dt-bindings/media/tda1997x.h

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@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 v3 01/11] dt-bindings: fix the binding of Allwinner DE2 CCU of A83T and H3
From: Rob Herring @ 2017-12-26 19:00 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Maxime Ripard, Chen-Yu Tsai, linux-clk,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, dri-devel
In-Reply-To: <20171222122243.25735-2-icenowy-h8G6r0blFSE@public.gmane.org>

On Fri, Dec 22, 2017 at 6:22 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> The DE2 CCU is different on A83T and H3 -- the parent of the clocks on
> A83T is PLL_DE but on H3 it's the DE module clock. This is not noticed
> when I develop the DE2 CCU driver.
>
> Fix the binding by using different compatibles for A83T and H3, adding
> notes for the PLL_DE usage on A83T, and change the binding example's
> compatible from A83T to H3 (as it specifies the DE module clock).
>
> Fixes: ed74f8a8a679 ("dt-bindings: add binding for the Allwinner DE2 CCU")
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/clock/sun8i-de2.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Please add acks when posting new versions.

Rob
--
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 0/3] mtd: spi-nor: fix DMA-unsafe buffer issue between MTD and SPI
From: Trent Piepho @ 2017-12-26 18:45 UTC (permalink / raw)
  To: linux-mtd@lists.infradead.org, linux@armlinux.org.uk,
	broonie@kernel.org, cyrille.pitchen@wedev4u.fr,
	dwmw2@infradead.org, computersforpeace@gmail.com,
	boris.brezillon@free-electrons.com, vigneshr@ti.com,
	richard@nod.at, marek.vasut@gmail.com
  Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	nicolas.ferre@microchip.com, robh@kernel.org,
	radu.pirea@microchip.com, devicetree@vger.kernel.org
In-Reply-To: <cover.1514087323.git.cyrille.pitchen@wedev4u.fr>

On Sun, 2017-12-24 at 05:36 +0100, Cyrille Pitchen wrote:
> this series tries to solve a long time issue of compatibility between the
> MTD and SPI sub-systems about whether we should use DMA-safe memory.

Can this should replace SoC specific fixes like:

c687c46e9e45
spi: spi-ti-qspi: Use bounce buffer if read buffer is not DMA'ble

7094576ccdc3
spi: atmel: fix corrupted data issue on SAM9 family SoCs

Or, since this only fixes instances of DMA-unsafe buffers used in
access to SPI NOR flash chips, and since there are other SPI master
interface users, those chip specific fixes in some/all spi master
drivers are still needed to fix transfers not originated via spi-nor? 
Or are all the previous fixes necessary because of spi-nor flash (via
ubifs or jffs2) and once that's fixed via this series there are no more
originators of dma-unsafe buffers?

> The SPI sub-system has already implemented a work-around on its side,
> based on the spi_map_buf() function. However this function has its own
> limitation too. Especially, even if it builds a 'struct scatterlist' from
> a vmalloc'ed buffer, calling dma_map_sg() is still not safe on all
> architectures. Especially, on ARM cores using either VIPT or VIVT data
> caches, dma_map_sg() doesn't take the cache aliases issue into account.
> Then numerous crashes were reported for such architectures.

Could the above issue also be fixed via calls to
flush_kernel_vmap_range() and invalidate_kernel_vmap_range() to keep
the data cache valid?  Or is there a reason that won't work?

^ permalink raw reply

* Re: [PATCH v3 2/3] dt-bindings: Add binding for Sitronix ST7735R display panels
From: Rob Herring @ 2017-12-26 18:39 UTC (permalink / raw)
  To: David Lechner
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, limor,
	linux-kernel@vger.kernel.org, dri-devel
In-Reply-To: <1513884233-31540-3-git-send-email-david@lechnology.com>

On Thu, Dec 21, 2017 at 1:23 PM, David Lechner <david@lechnology.com> wrote:
> This adds a new device tree binding for Sitronix ST7735R display panels,
> such as the Adafruit 1.8" TFT.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>
> v3 changes:
> * compatible string is changed from "sitronix,st7735r-jd-t18003-t01" to
> "jianda,jd-t18003-t01", "sitronix,st7735r"
>
> v2 changes:
> * none
>
>  .../bindings/display/sitronix,st7735r.txt          | 35 ++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sitronix,st7735r.txt

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2 5/7] dt-bindings: i3c: Document core bindings
From: Rob Herring @ 2017-12-26 18:29 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Wolfram Sang, linux-i2c, Jonathan Corbet, linux-doc,
	Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
	Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
	Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
	Thomas Petazzoni, Nishanth Menon, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, OPEN 
In-Reply-To: <20171221114144.2610d49b@bbrezillon>

On Thu, Dec 21, 2017 at 4:41 AM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Wed, 20 Dec 2017 12:06:45 -0600
> Rob Herring <robh@kernel.org> wrote:
>
>> On Sat, Dec 16, 2017 at 07:35:37PM +0100, Boris Brezillon wrote:
>> > On Sat, 16 Dec 2017 11:20:40 -0600
>> > Rob Herring <robh@kernel.org> wrote:
>> >
>> > > On Thu, Dec 14, 2017 at 04:16:08PM +0100, Boris Brezillon wrote:
>> > > > A new I3C subsystem has been added and a generic description has been
>> > > > created to represent the I3C bus and the devices connected on it.
>> > > >
>> > > > Document this generic representation.
>>
>> [...]
>>
>> > > So please define the node
>> > > name to be "i3c-controller". That's more inline with other node names
>> > > than i3c-master that you used below.
>> >
>> > Hm, not sure i3c-controller is appropriate though, because you can have
>> > slave controllers. Maybe i3c-host, but I'd prefer to keep the term
>> > master since it's employed everywhere in the spec. I can also be
>> > i3c-master-controller if you prefer.
>>
>> Okay, i3c-master is fine. Just make it explicit.
>
> Okay.
>
>>
>> > > > +I3C devices
>> > > > +===========
>> > > > +
>> > > > +All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
>> > > > +are thus discoverable. So, by default, I3C devices do not have to be described
>> > > > +in the device tree.
>> > > > +This being said, one might want to attach extra resources to these devices,
>> > > > +and those resources may have to be described in the device tree, which in turn
>> > > > +means we have to describe I3C devices.
>> > > > +
>> > > > +Another use case for describing an I3C device in the device tree is when this
>> > > > +I3C device has a static address and we want to assign it a specific dynamic
>> > > > +address before the DAA takes place (so that other devices on the bus can't
>> > > > +take this dynamic address).
>> > > > +
>> > > > +Required properties
>> > > > +-------------------
>> > > > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a
>> > > > +          device discovered during DAA with its device tree definition. The
>> > > > +          PID is supposed to be unique on a given bus, which guarantees a 1:1
>> > > > +          match. This property becomes optional if a reg property is defined,
>> > > > +          meaning that the device has a static address.
>> > >
>> > > What determines this number?
>> >
>> > Part of it is fixed (manufacturer and part id) and the last few bits
>> > represent the device instance on the bus (so you can have several
>> > identical devices on the same bus). The manufacturer and part ids
>> > should be statically assigned during production, instance id is usually
>> > configurable through extra pins that you drive high or low at reset
>> > time.
>>
>> Sounds like an I2C address at least for the pin strapping part...
>
> The address space of this instance-id is smaller (4bits) than the I2C
> one (7bits), and more importantly, the instance-id is not required to
> be unique, it's the aggregation of the vendor-id, part-id and
> instance-id that has to be unique. So, if you were thinking about using
> this id to uniquely identify the device on the bus it's not a good idea.

No, no. I was just commenting how I2C devices typically do the same
pin strapping to make addresses unique.


>> > > > +Optional properties
>> > > > +-------------------
>> > > > +- reg: static address. Only valid is the device has a static address.
>> > > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This
>> > > > +                      property depends on the reg property.
>> > >
>> > > Perhaps "assigned-address" property would be appropriate. I'm not all
>> > > that familiar with it though.
>> >
>> > Again, the spec use the term "dynamic address" everywhere, and I'd like
>> > to stay as close as possible to the spec.
>>
>> I looked at assigned-addresses a bit more and that won't really fit
>> because it should be the same format as reg. So I think reg should
>> always be the PID as that is fixed and always present. Then the DAA
>> address is separate and can be the i3c-dynamic-address property.
>>
>> However, there's still part I don't understand...
>>
>> > > > +               /* I3C device with a static address. */
>> > > > +               thermal_sensor: sensor@68 {
>> > > > +                       reg = <0x68>;
>> > > > +                       i3c-dynamic-address = <0xa>;
>>
>> I'm confused as to how/why you have both reg and dynamic address?
>
> Some I3C devices have an I2C address (also called static or legacy
> address in a few places). The static/I2C/legacy address is used until
> the I3C device is assigned a dynamic address by the master. The whole
> point of specifying both an I2C address (through the reg property) and
> a dynamic address (through the i3c-dynamic-address) is to tell the
> controller that a specific dynamic address should be assigned to this
> device using the SETSADA (Set Dynamic Address from Static Address)
> command before a DAA (Dynamic Address Assignment) procedure is started.
> This way, the device will not participate to the DAA (because it
> already has a valid DA) and the dynamic address can't be assigned to
> a different device (which is one of the problem with the automatic DAA
> procedure).

Okay, think I got it now.

I think we should extend "reg" to have either I2C address, I3C PID, or
both (in a defined order). I'm assuming you can always distinguish a
static I2C address and an I3C PID just by upper bits all being 0s for
I2C addresses. Maybe both is not needed? This means we'd have to allow
64-bit I2C addresses (#address-cells=2), but that should be easily
fixed if that causes problems in the kernel.

So i3c-pid would go away and i3c-dynamic-address stays.

Rob

^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
From: Tony Lindgren @ 2017-12-26 18:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: Kishon Vijay Abraham I, Lorenzo Pieralisi, Bjorn Helgaas,
	Mark Rutland, linux-omap,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sekhar Nori
In-Reply-To: <CAL_Jsq+f3u0yvftvESKDiDF76bUiwr9hZED3ZZ75=qMMr6RL1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

* Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [171226 17:49]:
> On Fri, Dec 22, 2017 at 12:24 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> [171222 06:06]:
> >> Hi Rob,
> >>
> >> On Thursday 21 December 2017 12:27 AM, Rob Herring wrote:
> >> > On Tue, Dec 19, 2017 at 02:28:22PM +0530, Kishon Vijay Abraham I wrote:
> >> >> Add syscon properties required for configuring PCIe in x2 lane mode.
> >> >>
> >> >> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> >> >> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> >> >> ---
> >> >>  Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++
> >> >>  1 file changed, 6 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> >> >> index 82cb875e4cec..bfbc77ac7355 100644
> >> >> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
> >> >> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> >> >> @@ -13,6 +13,12 @@ PCIe DesignWare Controller
> >> >>   - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
> >> >>           where <X> is the instance number of the pcie from the HW spec.
> >> >>   - num-lanes as specified in ../designware-pcie.txt
> >> >> + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control
> >> >> +                   module and the register offset to specify 1 lane or
> >> >> +                   2 lane.
> >> >> + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control
> >> >> +                  module and the register offset to specify lane
> >> >> +                  selection.
> >> >
> >> > Adding a property for every syscon register doesn't really scale and
> >> > doesn't work if the register layout changes.
> >>
> >> The register layout doesn't really change between silicon revisions and for new
> >> SoCs, the phandle and the register offset for that SoC will have to be
> >> populated again.
> 
> And what about SoCs that don't exist yet?
> 
> >> Having said that, I'm not aware of any other alternative here.
> 
> What would you do if you had 20 different syscon registers to
> configure? Add 20 properties? No, you would have per SoC functions in
> the driver to handle the different cases.

Ideally these syscon registers would be managed by some Linux
generic framework such as clock/regulator/mux/phy.

But yeah, if that does not work, then setting a SoC specific
configuration function based on the compatible value makes sense
to me.

> > Sorry I did not realize this is still open. Sounds like I need to
> > revert commit 4ece93c020e3 ("ARM: dts: dra7: Add properties to
> > enable PCIe x2 lane mode"), let me know if that is not the case.
> 
> It's fine, I guess. Keep adding more syscon phandles and then I'll NAK
> it (if I remember :)).

Already reverted, thanks for the comments.

Regards,

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

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx: Add memory node unit name
From: Fabio Estevam @ 2017-12-26 18:09 UTC (permalink / raw)
  To: Marco Franchi
  Cc: Lothar Waßmann, Shawn Guo,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, Rob Herring,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAM4PwSVzVn3=wWpFohw2q23Oiwp1FimCkdHPq=BfG-QzuuPtfQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

On Thu, Dec 21, 2017 at 12:26 PM, Marco Franchi <marcofrk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Lothar,
>
> 2017-12-21 6:07 GMT-02:00 Lothar Waßmann <LW-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>:
>> Hi,
>>
>> On Wed,  6 Dec 2017 13:59:49 -0200 Marco Franchi wrote:
>>> Fix the following warnings from dtc by adding the unit name to memory
>>> nodes:
>>>
>>> Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
>>>
>>> Converted using the following command:
>>>
>>> perl -p0777i -e 's/memory \{\n\t\treg = \<0x+([0-9a-f])/memory\@$1$\0000000 \{\n\t\treg = <0x$1/m' `find ./arch/arm/boot/dts -name "imx*"`
>>>
>>> The files below were manually fixed:
>>> -imx1-ads.dts
>>> -imx1-apf9328.dts
>>>
>> The imx*.dtsi files all have this:
>> |       memory { device_type = "memory"; reg = <0 0>; };
>> Thus you will end up with a 'memory' node with a reg = <0 0> entry and
>> an additional 'memory@...' node with the correct 'reg' values.
>
> You are right. The .dtb files were composed by two different memory nodes.
> Do you have some recommendation to take off this specific warning?

Currently the only dtc warnings we have with imx_v6_v7_defconfig are:

Warning (unit_address_vs_reg): Node /memory has a reg or ranges
property, but no unit name

What is the recommended way to fix these warnings?

Thanks
--
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 v3 1/4] dt-bindings: samsung: document bindings for Midas family boards
From: Rob Herring @ 2017-12-26 18:02 UTC (permalink / raw)
  To: Simon Shields
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Kukjin Kim,
	Krzysztof Kozlowski,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Marek Szyprowski, Bartłomiej Żołnierkiewicz
In-Reply-To: <20171221014246.GA23568-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>

On Wed, Dec 20, 2017 at 7:42 PM, Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org> wrote:
> Hi Rob,
>
> Thanks for the review.
>
> On Wed, Dec 20, 2017 at 12:17:59PM -0600, Rob Herring wrote:
>> On Mon, Dec 18, 2017 at 11:38:02PM +1100, Simon Shields wrote:
>> > Document GT-I9300, GT-I9305, GT-N7100, and GT-N7105 bindings, along
>> > with the shared "midas" binding.
>> >
>> > Signed-off-by: Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
>> > ---
>> >  Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt | 4 ++++
>> >  1 file changed, 4 insertions(+)
>>
>> My comment on v2 remains.
>
> Do you have any example of a better description? All the other ARM board
> descriptions seem similarly terse.
>
> Alternatively, maybe changing the compatible strings is a better
> solution? "samsung,n710x" for t0, "samsung,i9300" for m0, and
> "samsung,i9305" for m3?

Yes, it was the compatible string I was commenting on. I don't really
care about the description because for most boards I have no idea what
they are.

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

^ permalink raw reply

* Re: [PATCH v7] i2c: Add support for NXP PCA984x family.
From: Rob Herring @ 2017-12-26 17:58 UTC (permalink / raw)
  To: Adrian Fiergolski
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Peter Rosin,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <20171225212646.8062-1-adrian.fiergolski-vJEk5272eHo@public.gmane.org>

On Mon, Dec 25, 2017 at 3:26 PM, Adrian Fiergolski
<adrian.fiergolski-vJEk5272eHo@public.gmane.org> wrote:
> This patch extends the current i2c-mux-pca954x driver and adds support for
> a newer PCA984x family of the I2C switches and multiplexers from NXP.
>
> Signed-off-by: Adrian Fiergolski <adrian.fiergolski-vJEk5272eHo@public.gmane.org>
> ---
> Following Rob's and Peter's remarks, bindings contain now one valid
> combination of compatibles per line.
>
>  .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    | 13 ++++++--
>  drivers/i2c/muxes/Kconfig                          |  6 ++--
>  drivers/i2c/muxes/i2c-mux-pca954x.c                | 38 +++++++++++++++++++---
>  3 files changed, 48 insertions(+), 9 deletions(-)

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@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 net-next v7 2/2] net: ethernet: socionext: add AVE ethernet driver
From: David Miller @ 2017-12-26 17:58 UTC (permalink / raw)
  To: hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, andrew-g2DYL2Zd6BY,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A,
	jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1513854776-4149-3-git-send-email-hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>

From: Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
Date: Thu, 21 Dec 2017 20:12:56 +0900

> +	writel(AVE_TXDC_ADDR_START
> +		| (((priv->tx.ndesc * priv->desc_size) << 16) & AVE_TXDC_SIZE),
> +		priv->base + AVE_TXDC);
...
> +	writel(AVE_RXDC0_ADDR_START
> +	       | (((priv->rx.ndesc * priv->desc_size) << 16) & AVE_RXDC0_SIZE),
> +	       priv->base + AVE_RXDC0);
 ...
> +	cmdsts = AVE_STS_OWN | AVE_STS_1ST | AVE_STS_LAST
> +		| (skb->len & AVE_STS_PKTLEN_TX_MASK);

Please do not begin lines with operators, instead put them at the end
of the previous line, f.e.:

	cmdsts = AVE_STS_OWN | AVE_STS_1ST | AVE_STS_LAST |
		 (skb->len & AVE_STS_PKTLEN_TX_MASK);

Thank you.
--
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 2/4] dma: fsl-qdma: add devicetree documentation for qDMA driver.
From: Rob Herring @ 2017-12-26 17:56 UTC (permalink / raw)
  To: Wen He
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jiafei Pan,
	Jiaheng Fan
In-Reply-To: <DBXPR04MB47875F9351B430136E1995CE20D0-obFqC9UnrTsfYwEmCIcpDAfhPeD8jYilXA4E9RH9d+qIuWR1G4zioA@public.gmane.org>

On Thu, Dec 21, 2017 at 1:09 AM, Wen He <wen.he_1-3arQi8VN3Tc@public.gmane.org> wrote:
> Hi Rob,
>
>> -----Original Message-----
>> From: Rob Herring [mailto:robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
>> Sent: 2017年12月21日 2:43
>> To: Wen He <wen.he_1-3arQi8VN3Tc@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH 2/4] dma: fsl-qdma: add devicetree documentation for
>> qDMA driver.
>>
>> On Tue, Dec 19, 2017 at 02:41:57PM +0800, Wen He wrote:
>>
>> Need a commit message.
>>
>
> Got it, Thanks.
>
>> > Signed-off-by: Wen He <wen.he_1-3arQi8VN3Tc@public.gmane.org>
>> > ---
>> >  Documentation/devicetree/bindings/dma/fsl-qdma.txt | 42
>> > ++++++++++++++++++++++
>> >  1 file changed, 42 insertions(+)
>> >  create mode 100644
>> Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> > b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> > new file mode 100644
>> > index 000000000000..b076177b4863
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> > @@ -0,0 +1,42 @@
>> > +* Freescale queue Direct Memory Access Controller(qDMA) Controller
>> > +
>> > +  The qDMA controller transfers blocks of data between one source and
>> > + one or more
>>
>> Why the indentation?
>>
>
> I did it by referring to Documentation/devicetree/bindings/dma/fsl-edma.txt, is it ok?

No, please change.

>> > +destinations. The blocks of data transferred can be represented in
>> > +memory as contiguous or non-contiguous using scatter/gather table(s).
>> > +Channel virtualization is supported through enqueuing of DMA jobs to,
>> > +or dequeuing DMA jobs from, different work queues.
>> > +
>> > +* qDMA Controller
>> > +Required properties:
>> > +- compatible :
>>
>> Add "Should be one of:"
>>
>> > +   - "fsl,ls1021a-qdma",
>> > +   Or "fsl,ls1043a-qdma" followed by "fsl,ls1021a-qdma",
>>
>> Then remove the "Or" and replace " followed by" with a comma (like dts
>> source).
>>
>
> - compatible : Should be "fsl,ls1021a-qdma" or "fsl,ls1043a-qdma", "fsl,ls1021a-qdma"
> Is that ok?

No, each line should enumerate each valid value of compatible. Like this:

- compatible : Should be one of:
  - "fsl,ls1021a-qdma",
  - "fsl,ls1043a-qdma", "fsl,ls1021a-qdma"

>> > +- reg : Specifies base physical address(s) and size of the qDMA registers.
>> > +   The region is qDMA control register's address and size.
>> > +- interrupts : A list of interrupt-specifiers, one for each entry in
>> > +   interrupt-names.
>> > +- interrupt-names : Should contain:
>> > +   "qdma-error" - the error interrupt
>> > +   "qdma-queue" - the queue interrupt
>> > +- channels : Number of channels supported by the controller
>>
>> dma-channels is the standard name.
>>
>
> Okay, got it.
>
>> > +- queues : Number of queues supported by driver
>>
>> Needs a vendor prefix.
>>
>
> Where do I put the vendor prefix?

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

^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
From: Rob Herring @ 2017-12-26 17:46 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Kishon Vijay Abraham I, Lorenzo Pieralisi, Bjorn Helgaas,
	Mark Rutland, linux-omap,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-pci, linux-kernel@vger.kernel.org, Sekhar Nori
In-Reply-To: <20171222182448.GZ3875@atomide.com>

On Fri, Dec 22, 2017 at 12:24 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Kishon Vijay Abraham I <kishon@ti.com> [171222 06:06]:
>> Hi Rob,
>>
>> On Thursday 21 December 2017 12:27 AM, Rob Herring wrote:
>> > On Tue, Dec 19, 2017 at 02:28:22PM +0530, Kishon Vijay Abraham I wrote:
>> >> Add syscon properties required for configuring PCIe in x2 lane mode.
>> >>
>> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++
>> >>  1 file changed, 6 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> >> index 82cb875e4cec..bfbc77ac7355 100644
>> >> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
>> >> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> >> @@ -13,6 +13,12 @@ PCIe DesignWare Controller
>> >>   - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
>> >>           where <X> is the instance number of the pcie from the HW spec.
>> >>   - num-lanes as specified in ../designware-pcie.txt
>> >> + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control
>> >> +                   module and the register offset to specify 1 lane or
>> >> +                   2 lane.
>> >> + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control
>> >> +                  module and the register offset to specify lane
>> >> +                  selection.
>> >
>> > Adding a property for every syscon register doesn't really scale and
>> > doesn't work if the register layout changes.
>>
>> The register layout doesn't really change between silicon revisions and for new
>> SoCs, the phandle and the register offset for that SoC will have to be
>> populated again.

And what about SoCs that don't exist yet?

>> Having said that, I'm not aware of any other alternative here.

What would you do if you had 20 different syscon registers to
configure? Add 20 properties? No, you would have per SoC functions in
the driver to handle the different cases.

> Sorry I did not realize this is still open. Sounds like I need to
> revert commit 4ece93c020e3 ("ARM: dts: dra7: Add properties to
> enable PCIe x2 lane mode"), let me know if that is not the case.

It's fine, I guess. Keep adding more syscon phandles and then I'll NAK
it (if I remember :)).

Rob

^ permalink raw reply

* Re: [PATCH v5 15/15] devicetree: bindings: Document qcom,pvs
From: Rob Herring @ 2017-12-26 17:36 UTC (permalink / raw)
  To: Sricharan R
  Cc: Mark Rutland, Rafael J. Wysocki,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Viresh Kumar, Michael Turquette, linux-pm, Stephen Boyd,
	Russell King, linux-kernel@vger.kernel.org, David Brown,
	linux-arm-msm, Andy Gross, open list:ARM/QUALCOMM SUPPORT,
	linux-clk,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <864b6cfd-d3eb-d876-82e2-ebc79c5a6956@codeaurora.org>

On Thu, Dec 21, 2017 at 5:53 AM, Sricharan R <sricharan@codeaurora.org> wrote:
> Hi Rob,
>
> On 12/21/2017 2:48 AM, Rob Herring wrote:
>> On Wed, Dec 20, 2017 at 11:55:33AM +0530, Sricharan R wrote:
>>> Hi Viresh,
>>>
>>> On 12/20/2017 8:56 AM, Viresh Kumar wrote:
>>>> On 19-12-17, 21:25, Sricharan R wrote:
>>>>> +  cpu@0 {
>>>>> +          compatible = "qcom,krait";
>>>>> +          enable-method = "qcom,kpss-acc-v1";
>>>>> +          device_type = "cpu";
>>>>> +          reg = <0>;
>>>>> +          qcom,acc = <&acc0>;
>>>>> +          qcom,saw = <&saw0>;
>>>>> +          clocks = <&kraitcc 0>;
>>>>> +          clock-names = "cpu";
>>>>> +          cpu-supply = <&smb208_s2a>;
>>>>> +          operating-points-v2 = <&cpu_opp_table>;
>>>>> +  };
>>>>> +
>>>>> +  qcom,pvs {
>>>>> +          qcom,pvs-format-a;
>>>>> +  };
>>>>
>>>> Not sure what Rob is going to say on that :)
>>>>
>>>
>>>  Yes. Would be good to know the best way.
>>
>> Seems like this should be a property of an efuse node either implied by
>> the compatible or a separate property. What determines format A vs. B?
>>
>
>  Yes, this efuse registers are part of the eeprom (qfprom) tied to the soc.
>  So this property (details like bitfields and register offsets that it represents)
>  can be put soc specific and nvmem apis can be used to read
>  the registers. Does something like below look ok ?
>
>  qcom,pvs {
>         compatible = "qcom,pvs-ipq8064";
>         nvmem-cells = <&pvs_efuse>;
>  }

Why do you need this node? It doesn't look like it corresponds to a
h/w block. It looks like you are just creating it to instantiate a
driver.

>  qfprom: qfprom@700000 {
>         compatible      = "qcom,qfprom";

Either this or...

>         reg             = <0x00700000 0x1000>;
>         #address-cells  = <1>;
>         #size-cells     = <1>;
>         ranges;
>         pvs_efuse: pvs {

a compatible here should be specific enough so the OS can know what
the bits are.

>         reg = <0xc0 0x8>;
>         };
>  };

^ permalink raw reply

* Re: [PATCH v2 1/5] drm/panel: Add support for the EDT ETM0700G0BDH6
From: Rob Herring @ 2017-12-26 17:25 UTC (permalink / raw)
  To: Türk, Jan
  Cc: Mark Rutland, devicetree@vger.kernel.org, SZ Lin, David Airlie,
	Kevin Hilman, Russell King, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, Thierry Reding, Alexandre Belloni,
	Sascha Hauer, Greg Kroah-Hartman, Fabio Estevam, Maxime Ripard,
	Shawn Guo, Andreas Färber, LinuxArmKernelMailingListe
In-Reply-To: <95F51F4B902CAC40AF459205F6322F01B7FDFECC3D@BMK019S01.emtrion.local>

On Fri, Dec 22, 2017 at 4:43 AM, Türk, Jan <Jan.Tuerk@emtrion.de> wrote:
>> Von: Rob Herring [mailto:robh@kernel.org]
>> Gesendet: Freitag, 22. Dezember 2017 00:00
>> Betreff: Re: [PATCH v2 1/5] drm/panel: Add support for the EDT
>> ETM0700G0BDH6
>>
>> On Wed, Dec 20, 2017 at 02:47:01PM +0100, jan.tuerk@emtrion.com wrote:
>> > From: Jan Tuerk <jan.tuerk@emtrion.com>
>> >
>> > The Emerging Display Technology ETM0700G0BDH6 is exactly the same
>> > display as the ETM0700G0DH6, exept the pixelclock polarity. Therefore
>> > re-use the ETM0700G0DH6 modes. It is used by default on emtrion Avari
>> > based development kits.
>>
>> As I asked on v1, why not document the panels together in a single doc?
>
> As denoted in the cover letter:

I generally don't read cover letters...

>>The documentation for the EDT display is kept as an extra file currently,
>>as it is done by the most displays in the documentation. Also a new
>>new Variant of the EDT already arrived. So merging their documentations
>>should be discussed separately.

You mean a 3rd variant?

> I think it will be even a little tricky to find a matching filename for both versions,
> as the recent ones adding an extra character in the description. Are you expecting sth.
> like edt,etm0700series.txt?

Yeah, or edt,etm0700g0.txt.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [v3,2/2] hwmon: (aspeed-pwm-tacho) Deassert reset in probe
From: Guenter Roeck @ 2017-12-26 17:24 UTC (permalink / raw)
  To: Joel
  Cc: Rob Herring, Philipp Zabel, Mykola Kostenok,
	Jaghathiswari Rankappagounder Natarajan, Patrick Venture,
	Andrew Jeffery, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171223130528.5346-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Sat, Dec 23, 2017 at 11:35:28PM +1030, Joel wrote:
> The ASPEED SoC must deassert a reset in order to use the PWM/tach
> peripheral.
> 
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/aspeed-pwm-tacho.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
> index 63a95e23ca81..693a3d53cab5 100644
> --- a/drivers/hwmon/aspeed-pwm-tacho.c
> +++ b/drivers/hwmon/aspeed-pwm-tacho.c
> @@ -19,6 +19,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  #include <linux/sysfs.h>
>  #include <linux/thermal.h>
>  
> @@ -181,6 +182,7 @@ struct aspeed_cooling_device {
>  
>  struct aspeed_pwm_tacho_data {
>  	struct regmap *regmap;
> +	struct reset_control *rst;
>  	unsigned long clk_freq;
>  	bool pwm_present[8];
>  	bool fan_tach_present[16];
> @@ -905,6 +907,13 @@ static int aspeed_create_fan(struct device *dev,
>  	return 0;
>  }
>  
> +static void aspeed_pwm_tacho_remove(void *data)
> +{
> +	struct aspeed_pwm_tacho_data *priv = data;
> +
> +	reset_control_assert(priv->rst);
> +}
> +
>  static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -931,6 +940,19 @@ static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
>  			&aspeed_pwm_tacho_regmap_config);
>  	if (IS_ERR(priv->regmap))
>  		return PTR_ERR(priv->regmap);
> +
> +	priv->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	if (IS_ERR(priv->rst)) {
> +		dev_err(dev,
> +			"missing or invalid reset controller device tree entry");
> +		return PTR_ERR(priv->rst);
> +	}
> +	reset_control_deassert(priv->rst);
> +
> +	ret = devm_add_action_or_reset(dev, aspeed_pwm_tacho_remove, priv);
> +	if (ret)
> +		return ret;
> +
>  	regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE, 0);
>  	regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE_EXT, 0);
>  
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [v3,1/2] dt-bindings: hwmon: aspeed-pwm-tacho: Add reset node
From: Guenter Roeck @ 2017-12-26 17:24 UTC (permalink / raw)
  To: Joel
  Cc: Rob Herring, Philipp Zabel, Mykola Kostenok,
	Jaghathiswari Rankappagounder Natarajan, Patrick Venture,
	Andrew Jeffery, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171223130528.5346-2-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

On Sat, Dec 23, 2017 at 11:35:27PM +1030, Joel wrote:
> The device tree bindings are updated to document the resets phandle, and
> the example is updated to match what is expected for both the reset and
> clock phandle.
> 
> Note that the bindings should have always had the reset controller, as
> the hardware is unusable without it.
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt         | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> index 367c8203213b..3ac02988a1a5 100644
> --- a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> @@ -22,8 +22,9 @@ Required properties for pwm-tacho node:
>  - compatible : should be "aspeed,ast2400-pwm-tacho" for AST2400 and
>  	       "aspeed,ast2500-pwm-tacho" for AST2500.
>  
> -- clocks : a fixed clock providing input clock frequency(PWM
> -	   and Fan Tach clock)
> +- clocks : phandle to clock provider with the clock number in the second cell
> +
> +- resets : phandle to reset controller with the reset number in the second cell
>  
>  fan subnode format:
>  ===================
> @@ -48,19 +49,14 @@ Required properties for each child node:
>  
>  Examples:
>  
> -pwm_tacho_fixed_clk: fixedclk {
> -	compatible = "fixed-clock";
> -	#clock-cells = <0>;
> -	clock-frequency = <24000000>;
> -};
> -
>  pwm_tacho: pwmtachocontroller@1e786000 {
>  	#address-cells = <1>;
>  	#size-cells = <1>;
>  	#cooling-cells = <2>;
>  	reg = <0x1E786000 0x1000>;
>  	compatible = "aspeed,ast2500-pwm-tacho";
> -	clocks = <&pwm_tacho_fixed_clk>;
> +	clocks = <&syscon ASPEED_CLK_APB>;
> +	resets = <&syscon ASPEED_RESET_PWM>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>;
>  
--
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


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