* Re: [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Ray Jui @ 2018-05-23 19:29 UTC (permalink / raw)
To: Rob Herring
Cc: Robin Murphy, Wim Van Sebroeck, Guenter Roeck, Mark Rutland,
Frank Rowand, Catalin Marinas, Will Deacon, devicetree,
linux-watchdog, linux-kernel, bcm-kernel-feedback-list,
linux-arm-kernel
In-Reply-To: <20180523185944.GA9989@rob-hp-laptop>
On 5/23/2018 11:59 AM, Rob Herring wrote:
> On Wed, May 23, 2018 at 09:25:49AM -0700, Ray Jui wrote:
>>
>>
>> On 5/23/2018 3:57 AM, Robin Murphy wrote:
>>> On 22/05/18 19:47, Ray Jui wrote:
>>>> Update the SP805 binding document to add optional 'timeout-sec'
>>>> devicetree property
>>>>
>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>> ---
>>>> Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> index edc4f0e..f898a86 100644
>>>> --- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> @@ -19,6 +19,8 @@ Required properties:
>>>> Optional properties:
>>>> - interrupts : Should specify WDT interrupt number.
>>>> +- timeout-sec : Should specify default WDT timeout in seconds. If
>>>> unset, the
>>>> + default timeout is 30 seconds
>>>
>>> According to the SP805 TRM, the default interval is dependent on the
>>> rate of WDOGCLK, but would typically be a lot longer than that :/
>>>
>>> On a related note, anyone have any idea why we seem to have two subtly
>>> different SP805 bindings defined?
>
> Sigh.
>
>> Interesting, I did not even know that until you pointed this out (and it's
>> funny that I found that I actually reviewed arm,sp805.txt internally in
>> Broadcom code review).
>>
>> It looks like one was done by Bhupesh Sharma (sp805-wdt.txt) and the other
>> was done by Anup Patel (arm,sp805.txt). Both were merged at the same time
>> around March 20, 2016: 915c56bc01d6. I'd assume both were sent out at around
>> the same time.
>>
>> It sounds like we should definitely remove one of them. Given that
>> sp805-wdt.txt appears to have more detailed descriptions on the use of the
>> clocks, should we remove arm,sp805.txt?
>
> Take whichever text you like, but I prefer filenames using the
> compatible string and the correct string is 'arm,sp805' because '-wdt'
> is redundant. You can probably safely just update all the dts files with
> 'arm,sp805' and just remove 'arm,sp805-wdt' because it is not actually
> used (as the ID registers are).
Okay. I'll consolidate everything into arm,sp805.txt. Will also fix all
DTS files to use "arm,sp805". The fix for actual DTS files will be in a
different patch series.
>
> Rob
>
^ permalink raw reply
* Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings
From: Rob Herring @ 2018-05-23 19:28 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Greg Kroah-Hartman, Karthikeyan Ramasubramanian, sdharia, girishm,
linux-kernel@vger.kernel.org, Mark Rutland, Banajit Goswami,
devicetree, Mark Brown, linux-arm-msm, Linux-ALSA
In-Reply-To: <4b83b699-59ad-b177-f360-3e76ba847892@linaro.org>
On Wed, May 23, 2018 at 1:11 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
>
> On 23/05/18 17:40, Rob Herring wrote:
>>>
>>> +
>>> +- qcom,ngd-id
>>> + Usage: required
>>> + Value type: <u32>
>>> + Definition: ngd instance id in the controller
>>
>> Why do you need this?
>>
> Please ignore my comment from previous reply.
>
> There are more than one instances of ngd in this slim controller.
> We need this to make sure we are programming the correct one.
Doesn't the parent-child relationship of devices on the bus provide
that? If you mean to provide consistent numbering to userspace, then
that's not a DT problem (nor one that Linux plans to solve).
> We also need this instance ID during powering it up using QMI.
Wouldn't that be a QMI ID?
Rob
^ permalink raw reply
* Re: [PATCH V4 8/8] dt-bindings: stm32: add compatible for syscon
From: Rob Herring @ 2018-05-23 19:03 UTC (permalink / raw)
To: Christophe Roullier
Cc: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro,
devicetree, linux-arm-kernel, netdev, andrew
In-Reply-To: <1527090479-5263-9-git-send-email-christophe.roullier@st.com>
On Wed, May 23, 2018 at 05:47:59PM +0200, Christophe Roullier wrote:
> This patch describes syscon DT bindings.
>
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> ---
> Documentation/devicetree/bindings/arm/stm32.txt | 10 ----------
> .../devicetree/bindings/arm/stm32/stm32-syscon.txt | 14 ++++++++++++++
> Documentation/devicetree/bindings/arm/stm32/stm32.txt | 10 ++++++++++
> 3 files changed, 24 insertions(+), 10 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/arm/stm32.txt
> create mode 100644 Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
> create mode 100644 Documentation/devicetree/bindings/arm/stm32/stm32.txt
In the future, use the -M option so file moves don't show any diff.
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Rob Herring @ 2018-05-23 18:59 UTC (permalink / raw)
To: Ray Jui
Cc: Robin Murphy, Wim Van Sebroeck, Guenter Roeck, Mark Rutland,
Frank Rowand, Catalin Marinas, Will Deacon, devicetree,
linux-watchdog, linux-kernel, bcm-kernel-feedback-list,
linux-arm-kernel
In-Reply-To: <c542388f-e417-f624-ade1-95f9a2fc8a3b@broadcom.com>
On Wed, May 23, 2018 at 09:25:49AM -0700, Ray Jui wrote:
>
>
> On 5/23/2018 3:57 AM, Robin Murphy wrote:
> > On 22/05/18 19:47, Ray Jui wrote:
> > > Update the SP805 binding document to add optional 'timeout-sec'
> > > devicetree property
> > >
> > > Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> > > Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> > > ---
> > > Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > > b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > > index edc4f0e..f898a86 100644
> > > --- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > > +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> > > @@ -19,6 +19,8 @@ Required properties:
> > > Optional properties:
> > > - interrupts : Should specify WDT interrupt number.
> > > +- timeout-sec : Should specify default WDT timeout in seconds. If
> > > unset, the
> > > + default timeout is 30 seconds
> >
> > According to the SP805 TRM, the default interval is dependent on the
> > rate of WDOGCLK, but would typically be a lot longer than that :/
> >
> > On a related note, anyone have any idea why we seem to have two subtly
> > different SP805 bindings defined?
Sigh.
> Interesting, I did not even know that until you pointed this out (and it's
> funny that I found that I actually reviewed arm,sp805.txt internally in
> Broadcom code review).
>
> It looks like one was done by Bhupesh Sharma (sp805-wdt.txt) and the other
> was done by Anup Patel (arm,sp805.txt). Both were merged at the same time
> around March 20, 2016: 915c56bc01d6. I'd assume both were sent out at around
> the same time.
>
> It sounds like we should definitely remove one of them. Given that
> sp805-wdt.txt appears to have more detailed descriptions on the use of the
> clocks, should we remove arm,sp805.txt?
Take whichever text you like, but I prefer filenames using the
compatible string and the correct string is 'arm,sp805' because '-wdt'
is redundant. You can probably safely just update all the dts files with
'arm,sp805' and just remove 'arm,sp805-wdt' because it is not actually
used (as the ID registers are).
Rob
^ permalink raw reply
* Re: [RESEND PATCH 3/4] dt-bindings: PCI: cadence: Add DT bindings for optional PHYs
From: Rob Herring @ 2018-05-23 18:52 UTC (permalink / raw)
To: Alan Douglas
Cc: bhelgaas, kishon, lorenzo.pieralisi, linux-pci, devicetree,
linux-kernel, cyrille.pitchen
In-Reply-To: <1526995952-22031-1-git-send-email-adouglas@cadence.com>
On Tue, May 22, 2018 at 02:32:32PM +0100, Alan Douglas wrote:
> Update DT documentation to include optional PHYs for cadence PCIe
> host and endpoint controllers.
>
> Signed-off-by: Alan Douglas <adouglas@cadence.com>
> ---
> Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt | 4 ++++
> Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt
> index 9a305237..e40c635 100644
> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt
> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt
> @@ -9,6 +9,8 @@ Required properties:
>
> Optional properties:
> - max-functions: Maximum number of functions that can be configured (default 1).
> +- phys: From PHY bindings: List of Generic PHY phandles.
> +- phy-names: List of names to identify the PHY.
You need to define how many. I'd assume one, but I guess your example is
1 per lane.
>
> Example:
>
> @@ -19,4 +21,6 @@ pcie@fc000000 {
> reg-names = "reg", "mem";
> cdns,max-outbound-regions = <16>;
> max-functions = /bits/ 8 <8>;
> + phys = <&ep_phy0 &ep_phy1>;
> + phy-names = "pcie-lane0","pcie-lane1";
> };
> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
> index 20a33f3..c0ca4c1 100644
> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt
> @@ -24,6 +24,8 @@ Optional properties:
> translations (default 32)
> - vendor-id: The PCI vendor ID (16 bits, default is design dependent)
> - device-id: The PCI device ID (16 bits, default is design dependent)
> +- phys: From PHY bindings: List of Generic PHY phandles.
> +- phy-names: List of names to identify the PHY.
>
> Example:
>
> --
> 2.2.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC 1/3] scsi: ufs: set the device reference clock setting
From: Rob Herring @ 2018-05-23 18:48 UTC (permalink / raw)
To: Sayali Lokhande
Cc: subhashj, cang, vivek.gautam, rnayak, vinholikatti, jejb,
martin.petersen, asutoshd, linux-scsi, Mark Rutland,
Mathieu Malaterre,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <1526962900-20683-2-git-send-email-sayalil@codeaurora.org>
On Tue, May 22, 2018 at 09:51:38AM +0530, Sayali Lokhande wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
>
> UFS host supplies the reference clock to UFS device and UFS device
> specification allows host to provide one of the 4 frequencies (19.2 MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> device reference clock frequency setting in the device based on what
> frequency it is supplying to UFS device.
>
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> [cang@codeaurora.org: Resolved trivial merge conflicts]
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> ---
> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 8 +++
> drivers/scsi/ufs/ufs.h | 9 ++++
> drivers/scsi/ufs/ufshcd-pltfrm.c | 20 ++++++++
> drivers/scsi/ufs/ufshcd.c | 60 ++++++++++++++++++++++
> drivers/scsi/ufs/ufshcd.h | 2 +
> 5 files changed, 99 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..ac94220 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
> -lanes-per-direction : number of lanes available per direction - either 1 or 2.
> Note that it is assume same number of lanes is used both
> directions at once. If not specified, default is 2 lanes per direction.
> +- dev-ref-clk-freq : Specify the device reference clock frequency, must be one of the following:
> + 0: 19.2 MHz
> + 1: 26 MHz
> + 2: 38.4 MHz
> + 3: 52 MHz
> + Defaults to 26 MHz if not specified.
You already have "ref_clk", can't you just read its frequency?
Rob
^ permalink raw reply
* Re: [PATCH] drm/panel: simple: Add Sharp LQ035Q7DB03 panel support
From: Rob Herring @ 2018-05-23 18:39 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: David Airlie, devicetree, Thierry Reding, dri-devel
In-Reply-To: <20180521232314.5972-1-vz@mleia.com>
On Tue, May 22, 2018 at 02:23:14AM +0300, Vladimir Zapolskiy wrote:
> The change adds support for Sharp LQ035Q7DB03 3.5" QVGA panel.
>
> Note that this aged panel is already found in the kernel sources,
> for instance in board mach files mach-mx21ads.c, mach-mx27ads.c,
> mach-pcm043.c, lpd270.c and imx27-phytec-phycore-rdk.dts.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
> .../bindings/display/panel/sharp,lq035q7db03.txt | 7 ++++++
> drivers/gpu/drm/panel/panel-simple.c | 27 ++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,lq035q7db03.txt
>
> diff --git a/Documentation/devicetree/bindings/display/panel/sharp,lq035q7db03.txt b/Documentation/devicetree/bindings/display/panel/sharp,lq035q7db03.txt
> new file mode 100644
> index 0000000..42027e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq035q7db03.txt
> @@ -0,0 +1,7 @@
> +Sharp LQ035Q7DB03 3.5" QVGA TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "sharp,lq035q7db03"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
You need to state which properties from there this panel actually uses.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [RFC 12/13] ARM: dts: ti: add dra71-evm FIT description file
From: Tony Lindgren @ 2018-05-23 18:37 UTC (permalink / raw)
To: Tero Kristo
Cc: mark.rutland, Rob Herring, devicetree, Russell King - ARM Linux,
trini, frowand.list, wmills, linux-arm-kernel@lists.infradead.org
In-Reply-To: <243730d3-e7f3-584d-b5dd-491c2700cf11@ti.com>
* Tero Kristo <t-kristo@ti.com> [180523 05:58]:
> On 22/05/18 23:01, Rob Herring wrote:
> > On Mon, May 21, 2018 at 09:57:54AM +0300, Tero Kristo wrote:
> > > Ok trying to resurrect this thread a bit. Is there any kind of consensus how
> > > things like this should be handled? Should we add the DT overlay files to
> > > kernel tree or not?
> >
> > IMO, yes.
I too agree the overlay files should be in the kernel rather than
people having to try to figure out where to find them.
No idea how we'll verify they'll build though :) Maybe we
just need randconfig support for DT overlay files too?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH] dt-bindings: Add vendor prefix for Logic PD
From: Rob Herring @ 2018-05-23 18:35 UTC (permalink / raw)
To: Vladimir Zapolskiy; +Cc: devicetree
In-Reply-To: <20180521215946.3142-1-vz@mleia.com>
On Tue, May 22, 2018 at 12:59:46AM +0300, Vladimir Zapolskiy wrote:
> Logic PD is an electronics design, engineering and manufacturing
> company, which offers system on module and baseboard products.
>
> Website: https://www.logicpd.com/
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)
Applied, thanks.
Rob
^ permalink raw reply
* Re: [PATCH] iommu/ipmmu-vmsa: Document R-Car V3H and E3 IPMMU DT bindings
From: Rob Herring @ 2018-05-23 18:34 UTC (permalink / raw)
To: Magnus Damm
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ
In-Reply-To: <152691369384.29456.13581918319106400529.sendpatchset@little-apple>
On Mon, May 21, 2018 at 11:41:33PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
>
> Update the IPMMU DT binding documentation to include the compat strings
> for the IPMMU devices included in the R-Car V3H and E3 SoCs.
>
> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> ---
>
> Developed on top of renesas-drivers-2018-05-15-v4.17-rc5
>
> Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
^ permalink raw reply
* Re: [PATCH] clk: Add driver for MAX9485
From: Rob Herring @ 2018-05-23 18:33 UTC (permalink / raw)
To: Daniel Mack; +Cc: mturquette, sboyd, linux-clk, devicetree, Daniel Mack
In-Reply-To: <20180521085848.17192-1-daniel@zonque.org>
On Mon, May 21, 2018 at 10:58:48AM +0200, Daniel Mack wrote:
> From: Daniel Mack <zonque@gmail.com>
>
> This patch adds a driver for MAX9485, a programmable audio clock generator.
>
> The device requires a 27.000 MHz clock input. It can provide a gated
> buffered output of its input clock and two gated outputs of a PLL that can
> generate one out of 16 discrete frequencies. There is only one PLL however,
> so the two gated outputs will always have the same frequency but they can
> be switched individually.
>
> The driver for this device exposes 4 clocks in total:
>
> - MAX9485_MCLKOUT: A gated, buffered output of the input clock
> - MAX9485_CLKOUT: A PLL that can be configured to 16 different
> discrete frequencies
> - MAX9485_CLKOUT[1,2]: Two gated outputs for MAX9485_CLKOUT
>
> Some PLL output frequencies can be achieved with different register
> settings. The driver will select the one with lowest jitter in such cases.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> .../devicetree/bindings/clock/maxim,max9485.txt | 59 ++++
> drivers/clk/Kconfig | 8 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-max9485.c | 379 +++++++++++++++++++++
> include/dt-bindings/clock/maxim,max9485.h | 19 ++
The 2 DT files should be a separate patch.
> 5 files changed, 466 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/maxim,max9485.txt
> create mode 100644 drivers/clk/clk-max9485.c
> create mode 100644 include/dt-bindings/clock/maxim,max9485.h
>
> diff --git a/Documentation/devicetree/bindings/clock/maxim,max9485.txt b/Documentation/devicetree/bindings/clock/maxim,max9485.txt
> new file mode 100644
> index 000000000000..43713031be2c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/maxim,max9485.txt
> @@ -0,0 +1,59 @@
> +Devicetree bindings for Maxim MAX9485 Programmable Audio Clock Generator
> +
> +The driver for this device exposes 4 clocks in total:
Bindings describe h/w, not drivers.
> +- MAX9485_MCLKOUT: A gated, buffered output of the input clock of 27 MHz
> +- MAX9485_CLKOUT: A PLL that can be configured to 16 different discrete
> + frequencies
> +- MAX9485_CLKOUT[1,2]: Two gated outputs for MAX9485_CLKOUT
> +
> +MAX9485_CLKOUT[1,2] are children of MAX9485_CLKOUT which upchain all rate set
> +requests.
> +
> +Required properties:
> +- compatible: "maxim,max9485"
> +- clocks: Input clock, must provice 27.000 MHz
> +- clock-names: Must be set to "xclk"
> +- #clock-cells: From common clock binding; shall be set to 1
> +
> +Optional properties:
> +- reset-gpio: GPIO descriptor connected to the #RESET input pin
reset-gpios
> +- vdd-supply: A regulator node for Vdd
> +- clock-output-names: Name of output clocks, as defined in common clock
> + bindings
> +
> +If not explicitly set, the output names are "mclkout", "clkout", "clkout1"
> +and "clkout2".
> +
> +Clocks are defined as preprocessor macros in the dt-binding header.
> +
> +Example:
> +
> + #include <dt-bindings/clock/maxim,max9485.h>
> +
> + xo_27mhz: xo_27mhz {
Don't use '_' in node names.
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <27000000>;
> + };
> +
> + &i2c0 {
> + max9485: max9485@63 {
clock-controller@63
> + compatible = "maxim,max9485";
> + clock-names = "xclk";
> + clocks = <&xo_27mhz>;
> + reg = <0x63>;
> + reset-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
> + vdd-supply = <&3v3_reg>;
> + #clock-cells = <1>;
> + };
> + };
> +
> + // Clock consumer node
> +
> + foo@0 {
> + compatible = "bar,foo";
> + /* ... */
> + clock-names = "foo-intput-clk";
> + clocks = <&max9485 MAX9485_CLKOUT1>;
> + };
> diff --git a/include/dt-bindings/clock/maxim,max9485.h b/include/dt-bindings/clock/maxim,max9485.h
> new file mode 100644
> index 000000000000..23add67f32a7
> --- /dev/null
> +++ b/include/dt-bindings/clock/maxim,max9485.h
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Daniel Mack
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
Don't need this with the SPDX tag.
> + *
> + */
> +
> +#ifndef __DT_BINDINGS_MAX9485_CLK_H
> +#define __DT_BINDINGS_MAX9485_CLK_H
> +
> +#define MAX9485_MCLKOUT 0
> +#define MAX9485_CLKOUT 1
> +#define MAX9485_CLKOUT1 2
> +#define MAX9485_CLKOUT2 3
> +
> +#endif /* __DT_BINDINGS_MAX9485_CLK_H */
> --
> 2.14.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] dt-bindings: soc: update MT6797 power domain
From: Rob Herring @ 2018-05-23 18:27 UTC (permalink / raw)
To: KaiChieh Chuang
Cc: matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
mars.cheng-NuS5LvNUpcJWk0Htik3J/w,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wsd_upstream-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1526890186-16219-1-git-send-email-kaichieh.chuang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On Mon, May 21, 2018 at 04:09:46PM +0800, KaiChieh Chuang wrote:
> remove unused power domain in mt6797 and re-index
>
> Signed-off-by: KaiChieh Chuang <kaichieh.chuang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Mars Cheng <mars.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> include/dt-bindings/power/mt6797-power.h | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/include/dt-bindings/power/mt6797-power.h b/include/dt-bindings/power/mt6797-power.h
> index a60c1d8..f96c4e2 100644
> --- a/include/dt-bindings/power/mt6797-power.h
> +++ b/include/dt-bindings/power/mt6797-power.h
> @@ -20,11 +20,6 @@
> #define MT6797_POWER_DOMAIN_MM 3
> #define MT6797_POWER_DOMAIN_AUDIO 4
> #define MT6797_POWER_DOMAIN_MFG_ASYNC 5
> -#define MT6797_POWER_DOMAIN_MFG 6
> -#define MT6797_POWER_DOMAIN_MFG_CORE0 7
> -#define MT6797_POWER_DOMAIN_MFG_CORE1 8
> -#define MT6797_POWER_DOMAIN_MFG_CORE2 9
> -#define MT6797_POWER_DOMAIN_MFG_CORE3 10
> -#define MT6797_POWER_DOMAIN_MJC 11
> +#define MT6797_POWER_DOMAIN_MJC 6
This is an ABI. You can't just change the numbering. You'll cause a
mismatch between existing dtb and new kernel.
Rob
^ permalink raw reply
* Re: [PATCH 2/2] mtd: partitions: use DT info for parsing partitions with specified type
From: Boris Brezillon @ 2018-05-23 18:24 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
Marek Vasut, Rob Herring, linux-mtd, Jonas Gorski,
Rafał Miłecki, Brian Norris, David Woodhouse
In-Reply-To: <20180523171448.26234-2-zajec5@gmail.com>
On Wed, 23 May 2018 19:14:48 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This supports nested partitions in a DT. If selected partition has a
> "compatible" property specified it will be parsed looking for
> subpartitions.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> drivers/mtd/mtdpart.c | 33 +++++++++++++--------------------
> 1 file changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index f8d3a015cdad..52e2cb35fc79 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -322,22 +322,6 @@ static inline void free_partition(struct mtd_part *p)
> kfree(p);
> }
>
> -/**
> - * mtd_parse_part - parse MTD partition looking for subpartitions
> - *
> - * @slave: part that is supposed to be a container and should be parsed
> - * @types: NULL-terminated array with names of partition parsers to try
> - *
> - * Some partitions are kind of containers with extra subpartitions (volumes).
> - * There can be various formats of such containers. This function tries to use
> - * specified parsers to analyze given partition and registers found
> - * subpartitions on success.
> - */
> -static int mtd_parse_part(struct mtd_part *slave, const char *const *types)
> -{
> - return parse_mtd_partitions(&slave->mtd, types, NULL);
> -}
> -
> static struct mtd_part *allocate_partition(struct mtd_info *parent,
> const struct mtd_partition *part, int partno,
> uint64_t cur_offset)
> @@ -735,8 +719,8 @@ int add_mtd_partitions(struct mtd_info *master,
>
> add_mtd_device(&slave->mtd);
> mtd_add_partition_attrs(slave);
> - if (parts[i].types)
> - mtd_parse_part(slave, parts[i].types);
> + /* Look for subpartitions */
> + parse_mtd_partitions(&slave->mtd, parts[i].types, NULL);
>
> cur_offset = slave->offset + slave->mtd.size;
> }
> @@ -812,6 +796,12 @@ static const char * const default_mtd_part_types[] = {
> NULL
> };
>
> +/* Check DT only when looking for subpartitions. */
> +static const char * const default_subpartition_types[] = {
> + "ofpart",
> + NULL
> +};
> +
> static int mtd_part_do_parse(struct mtd_part_parser *parser,
> struct mtd_info *master,
> struct mtd_partitions *pparts,
> @@ -882,7 +872,9 @@ static int mtd_part_of_parse(struct mtd_info *master,
> const char *fixed = "fixed-partitions";
> int ret, err = 0;
>
> - np = of_get_child_by_name(mtd_get_of_node(master), "partitions");
> + np = mtd_get_of_node(master);
> + if (!mtd_is_partition(master))
> + np = of_get_child_by_name(np, "partitions");
> of_property_for_each_string(np, "compatible", prop, compat) {
> parser = mtd_part_get_compatible_parser(compat);
> if (!parser)
> @@ -945,7 +937,8 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
> int ret, err = 0;
>
> if (!types)
> - types = default_mtd_part_types;
> + types = mtd_is_partition(master) ? default_subpartition_types :
> + default_mtd_part_types;
Hm, that means the subparts inherit the parser types from their parent
if types != NULL? Is that really what we want? And if that's what we
want, why don't we do the same for types == NULL?
>
> for ( ; *types; types++) {
> /*
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH 05/15] drm/sun4i: Add TCON TOP driver
From: Rob Herring @ 2018-05-23 18:23 UTC (permalink / raw)
To: Jernej Skrabec
Cc: maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ, wens-jdAy2FN1RRM,
mark.rutland-5wv7dgnIgG8,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180519183127.2718-6-jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
On Sat, May 19, 2018 at 08:31:17PM +0200, Jernej Skrabec wrote:
> As already described in DT binding, TCON TOP is responsible for
> configuring display pipeline. In this initial driver focus is on HDMI
> pipeline, so TVE and LCD configuration is not implemented.
>
> Implemented features:
> - HDMI source selection
> - clock driver (TCON and DSI gating)
> - connecting mixers and TCONS
>
> Something similar also existed in previous SoCs, except that it was part
> of first TCON.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> ---
> drivers/gpu/drm/sun4i/Makefile | 3 +-
> drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 256 +++++++++++++++++++++
> drivers/gpu/drm/sun4i/sun8i_tcon_top.h | 20 ++
> include/dt-bindings/clock/sun8i-tcon-top.h | 11 +
This belongs with the binding doc patch.
> 4 files changed, 289 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h
> create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h
^ permalink raw reply
* Re: [PATCH 03/15] clk: sunxi-ng: r40: Export video PLLs
From: Rob Herring @ 2018-05-23 18:20 UTC (permalink / raw)
To: Jernej Skrabec
Cc: maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ, wens-jdAy2FN1RRM,
mark.rutland-5wv7dgnIgG8,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180519183127.2718-4-jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
On Sat, May 19, 2018 at 08:31:15PM +0200, Jernej Skrabec wrote:
> Video PLLs need to be referenced in R40 DT as possible HDMI PHY parent.
>
> Export them.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> ---
> drivers/clk/sunxi-ng/ccu-sun8i-r40.h | 8 ++++++--
> include/dt-bindings/clock/sun8i-r40-ccu.h | 4 ++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
^ permalink raw reply
* Re: [PATCH 06/11] dts: bindings: Restrict coresight tmc-etr scatter-gather mode
From: Rob Herring @ 2018-05-23 18:18 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: linux-arm-kernel, linux-kernel, mathieu.poirier, sudeep.holla,
frowand.list, coresight, mark.rutland, Mike Leach, John Horley,
Robert Walker, devicetree
In-Reply-To: <1526661567-4578-7-git-send-email-suzuki.poulose@arm.com>
On Fri, May 18, 2018 at 05:39:22PM +0100, Suzuki K Poulose wrote:
> We are about to add the support for ETR builtin scatter-gather mode
> for dealing with large amount of trace buffers. However, on some of
> the platforms, using the ETR SG mode can lock up the system due to
> the way the ETR is connected to the memory subsystem.
>
> In SG mode, the ETR performs READ from the scatter-gather table to
> fetch the next page and regular WRITE of trace data. If the READ
> operation doesn't complete(due to the memory subsystem issues,
> which we have seen on a couple of platforms) the trace WRITE
> cannot proceed leading to issues. So, we by default do not
> use the SG mode, unless it is known to be safe on the platform.
> We define a DT property for the TMC node to specify whether we
> have a proper SG mode.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: John Horley <john.horley@arm.com>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: frowand.list@gmail.com
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
> drivers/hwtracing/coresight/coresight-tmc.c | 9 ++++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC V2 2/6] hwmon: Add support for RPi voltage sensor
From: Guenter Roeck @ 2018-05-23 18:12 UTC (permalink / raw)
To: Robin Murphy
Cc: Stefan Wahren, Mark Rutland, Jean Delvare, Scott Branden,
Jonathan Corbet, Ray Jui, linux-doc, Phil Elwell, Eric Anholt,
devicetree, Rob Herring, bcm-kernel-feedback-list,
linux-rpi-kernel, Florian Fainelli, linux-hwmon, linux-arm-kernel,
Noralf Trønnes
In-Reply-To: <cbbdab47-cde3-7a7c-c797-c00d546e00d5@arm.com>
On Wed, May 23, 2018 at 01:12:10PM +0100, Robin Murphy wrote:
> On 22/05/18 20:31, Stefan Wahren wrote:
> [...]
> >>>>>+static int rpi_hwmon_probe(struct platform_device *pdev)
> >>>>>+{
> >>>>>+ struct device *dev = &pdev->dev;
> >>>>>+ struct rpi_hwmon_data *data;
> >>>>>+ int ret;
> >>>>>+
> >>>>>+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >>>>>+ if (!data)
> >>>>>+ return -ENOMEM;
> >>>>>+
> >>>>>+ data->fw = platform_get_drvdata(to_platform_device(dev->parent));
> >>>>>+ if (!data->fw)
> >>>>>+ return -EPROBE_DEFER;
> >>>>>+
> >>>>
> >>>>I am a bit at loss here (and sorry I didn't bring this up before).
> >>>>How would this ever be possible, given that the driver is registered
> >>>>from the firmware driver ?
> >>>
> >>>Do you refer to the (wrong) return code, the assumption that the parent must be a platform driver or a possible race?
> >>>
> >>
> >>The return code is one thing. My question was how the driver would ever be instantiated
> >>with platform_get_drvdata(to_platform_device(dev->parent)) == NULL (but dev->parent != NULL),
> >>so I referred to the race. But, sure, a second question would be how that would indicate
> >>that the parent is not instantiated yet (which by itself seems like an odd question).
> >
> >This shouldn't happen and worth a log error. In patch #3 the registration is called after the complete private data of the firmware driver is initialized. Did i missed something?
> >
> >But i must confess that i didn't test all builtin/module combinations.
>
> The point is that, by construction, a "raspberrypi-hwmon" device will only
> ever be created for this driver to bind to if the firmware device is both
> fully initialised and known to support the GET_THROTTLED call already. Thus
> trying to check those again from the hwmon driver is at best pointless, and
> at worst misleading. If somebody *does* manage to bind this driver to some
> random inappropriate device, you've still got no guarantee that dev->parent
> is valid or that dev_get_drvdata(dev->parent)) won't return something
> non-NULL that isn't a struct rpi_firmware pointer, at which point you're
> liable to pass the paranoid check yet still crash anyway.
>
> IOW, you can't reasonably defend against incorrect operation, and under
> correct operation there's nothing to defend against, so either way it's
> pretty futile to waste effort trying.
>
Well said.
Guenter
^ permalink raw reply
* Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings
From: Srinivas Kandagatla @ 2018-05-23 18:11 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, devicetree, alsa-devel, girishm, gregkh, broonie,
linux-kernel, bgoswami, kramasub, linux-arm-msm, sdharia
In-Reply-To: <20180523164023.GA24671@rob-hp-laptop>
On 23/05/18 17:40, Rob Herring wrote:
>> +
>> +- qcom,ngd-id
>> + Usage: required
>> + Value type: <u32>
>> + Definition: ngd instance id in the controller
> Why do you need this?
>
Please ignore my comment from previous reply.
There are more than one instances of ngd in this slim controller.
We need this to make sure we are programming the correct one.
We also need this instance ID during powering it up using QMI.
thanks,
srini
^ permalink raw reply
* Re: [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Guenter Roeck @ 2018-05-23 18:10 UTC (permalink / raw)
To: Robin Murphy
Cc: Ray Jui, Wim Van Sebroeck, Rob Herring, Mark Rutland,
Frank Rowand, Catalin Marinas, Will Deacon, devicetree,
linux-watchdog, linux-kernel, bcm-kernel-feedback-list,
linux-arm-kernel
In-Reply-To: <af81ea74-fb80-11e2-7bdc-d3607bdbd46b@arm.com>
On Wed, May 23, 2018 at 11:57:25AM +0100, Robin Murphy wrote:
> On 22/05/18 19:47, Ray Jui wrote:
> >Update the SP805 binding document to add optional 'timeout-sec'
> >devicetree property
> >
> >Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >---
> > Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> >index edc4f0e..f898a86 100644
> >--- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> >+++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> >@@ -19,6 +19,8 @@ Required properties:
> > Optional properties:
> > - interrupts : Should specify WDT interrupt number.
> >+- timeout-sec : Should specify default WDT timeout in seconds. If unset, the
> >+ default timeout is 30 seconds
>
> According to the SP805 TRM, the default interval is dependent on the rate of
> WDOGCLK, but would typically be a lot longer than that :/
>
Depends on the definition of "default". In the context of watchdog drivers,
it is (or should be) a driver default, not a chip default.
Guenter
> On a related note, anyone have any idea why we seem to have two subtly
> different SP805 bindings defined?
>
> Robin.
>
> > Examples:
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Guenter Roeck @ 2018-05-23 18:09 UTC (permalink / raw)
To: Robin Murphy
Cc: Ray Jui, Scott Branden, Mark Rutland, devicetree, linux-watchdog,
Catalin Marinas, Will Deacon, linux-kernel, Rob Herring,
bcm-kernel-feedback-list, Wim Van Sebroeck, Frank Rowand,
linux-arm-kernel
In-Reply-To: <ef3539fd-c95c-f364-93c7-680aabd1eea3@arm.com>
On Wed, May 23, 2018 at 06:15:14PM +0100, Robin Murphy wrote:
> On 23/05/18 17:29, Ray Jui wrote:
> >Hi Robin,
> >
> >On 5/23/2018 4:48 AM, Robin Murphy wrote:
> >>On 23/05/18 08:52, Scott Branden wrote:
> >>>
> >>>
> >>>On 18-05-22 04:24 PM, Ray Jui wrote:
> >>>>Hi Guenter,
> >>>>
> >>>>On 5/22/2018 1:54 PM, Guenter Roeck wrote:
> >>>>>On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
> >>>>>>If the watchdog hardware is already enabled during the boot process,
> >>>>>>when the Linux watchdog driver loads, it should reset the
> >>>>>>watchdog and
> >>>>>>tell the watchdog framework. As a result, ping can be generated from
> >>>>>>the watchdog framework, until the userspace watchdog daemon
> >>>>>>takes over
> >>>>>>control
> >>>>>>
> >>>>>>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>>>>>Reviewed-by: Vladimir Olovyannikov
> >>>>>><vladimir.olovyannikov@broadcom.com>
> >>>>>>Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >>>>>>---
> >>>>>> drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
> >>>>>> 1 file changed, 22 insertions(+)
> >>>>>>
> >>>>>>diff --git a/drivers/watchdog/sp805_wdt.c
> >>>>>>b/drivers/watchdog/sp805_wdt.c
> >>>>>>index 1484609..408ffbe 100644
> >>>>>>--- a/drivers/watchdog/sp805_wdt.c
> >>>>>>+++ b/drivers/watchdog/sp805_wdt.c
> >>>>>>@@ -42,6 +42,7 @@
> >>>>>> /* control register masks */
> >>>>>> #define INT_ENABLE (1 << 0)
> >>>>>> #define RESET_ENABLE (1 << 1)
> >>>>>>+ #define ENABLE_MASK (INT_ENABLE | RESET_ENABLE)
> >>>>>> #define WDTINTCLR 0x00C
> >>>>>> #define WDTRIS 0x010
> >>>>>> #define WDTMIS 0x014
> >>>>>>@@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
> >>>>>> MODULE_PARM_DESC(nowayout,
> >>>>>> "Set to 1 to keep watchdog running after device release");
> >>>>>> +/* returns true if wdt is running; otherwise returns false */
> >>>>>>+static bool wdt_is_running(struct watchdog_device *wdd)
> >>>>>>+{
> >>>>>>+ struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
> >>>>>>+
> >>>>>>+ if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
> >>>>>>+ ENABLE_MASK)
> >>>>>>+ return true;
> >>>>>>+ else
> >>>>>>+ return false;
> >>>>>
> >>>>> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
> >>>>>
> >>>>
> >>>>Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
> >>>>therefore, a simple !!(expression) would not work? That is, the
> >>>>masked result needs to be compared against the mask again to ensure
> >>>>both bits are set, right?
> >>>Ray - your original code looks correct to me. Easier to read and less
> >>>prone to errors as shown in the attempted translation to a single
> >>>statement.
> >>
> >> if (<boolean condition>)
> >> return true;
> >> else
> >> return false;
> >>
> >>still looks really dumb, though, and IMO is actually harder to read than
> >>just "return <boolean condition>;" because it forces you to stop and
> >>double-check that the logic is, in fact, only doing the obvious thing.
> >
> >If you can propose a way to modify my original code above to make it more
> >readable, I'm fine to make the change.
>
> Well,
>
> return readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK == ENABLE_MASK;
>
> would probably be reasonable to anyone other than the 80-column zealots, but
> removing the silly boolean-to-boolean translation idiom really only
> emphasises the fact that it's fundamentally a big complex statement; for
> maximum clarity I'd be inclined to separate the two logical operations (read
> and comparison), e.g.:
>
> u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
>
> return wdtcontrol & ENABLE_MASK == ENABLE_MASK;
== has higher precendence than bitwise &, so this will need ( ),
but otherwise I agree.
>
> which is still -3 lines vs. the original.
>
> >As I mentioned, I don't think the following change proposed by Guenter
> >will work due to the reason I pointed out:
> >
> >return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>
> FWIW, getting the desired result should only need one logical not swapping
> for a bitwise one there:
>
> return !(~readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK);
>
> but that's well into "too clever for its own good" territory ;)
Yes, that would be confusing.
>
> Robin.
^ permalink raw reply
* Re: [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Guenter Roeck @ 2018-05-23 18:06 UTC (permalink / raw)
To: Robin Murphy
Cc: Scott Branden, Ray Jui, Mark Rutland, devicetree, linux-watchdog,
Catalin Marinas, Will Deacon, linux-kernel, Rob Herring,
bcm-kernel-feedback-list, Wim Van Sebroeck, Frank Rowand,
linux-arm-kernel
In-Reply-To: <76d47e02-7a5f-3fc2-3905-cd4aa03ac69c@arm.com>
On Wed, May 23, 2018 at 12:48:10PM +0100, Robin Murphy wrote:
> On 23/05/18 08:52, Scott Branden wrote:
> >
> >
> >On 18-05-22 04:24 PM, Ray Jui wrote:
> >>Hi Guenter,
> >>
> >>On 5/22/2018 1:54 PM, Guenter Roeck wrote:
> >>>On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
> >>>>If the watchdog hardware is already enabled during the boot process,
> >>>>when the Linux watchdog driver loads, it should reset the watchdog and
> >>>>tell the watchdog framework. As a result, ping can be generated from
> >>>>the watchdog framework, until the userspace watchdog daemon takes over
> >>>>control
> >>>>
> >>>>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>>>Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> >>>>Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >>>>---
> >>>> drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
> >>>> 1 file changed, 22 insertions(+)
> >>>>
> >>>>diff --git a/drivers/watchdog/sp805_wdt.c
> >>>>b/drivers/watchdog/sp805_wdt.c
> >>>>index 1484609..408ffbe 100644
> >>>>--- a/drivers/watchdog/sp805_wdt.c
> >>>>+++ b/drivers/watchdog/sp805_wdt.c
> >>>>@@ -42,6 +42,7 @@
> >>>> /* control register masks */
> >>>> #define INT_ENABLE (1 << 0)
> >>>> #define RESET_ENABLE (1 << 1)
> >>>>+ #define ENABLE_MASK (INT_ENABLE | RESET_ENABLE)
> >>>> #define WDTINTCLR 0x00C
> >>>> #define WDTRIS 0x010
> >>>> #define WDTMIS 0x014
> >>>>@@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
> >>>> MODULE_PARM_DESC(nowayout,
> >>>> "Set to 1 to keep watchdog running after device release");
> >>>> +/* returns true if wdt is running; otherwise returns false */
> >>>>+static bool wdt_is_running(struct watchdog_device *wdd)
> >>>>+{
> >>>>+ struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
> >>>>+
> >>>>+ if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
> >>>>+ ENABLE_MASK)
> >>>>+ return true;
> >>>>+ else
> >>>>+ return false;
> >>>
> >>> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
> >>>
> >>
> >>Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
> >>therefore, a simple !!(expression) would not work? That is, the masked
> >>result needs to be compared against the mask again to ensure both bits
> >>are set, right?
> >Ray - your original code looks correct to me. Easier to read and less
> >prone to errors as shown in the attempted translation to a single
> >statement.
>
> if (<boolean condition>)
> return true;
> else
> return false;
>
> still looks really dumb, though, and IMO is actually harder to read than
> just "return <boolean condition>;" because it forces you to stop and
> double-check that the logic is, in fact, only doing the obvious thing.
>
Yes, and I won't accept it, sorry.
Guenter
> Robin.
>
>
>
> p.s. No thanks for making me remember the mind-boggling horror of briefly
> maintaining part of this legacy codebase... :P
>
> $ grep -r '? true : false' --include=*.cpp . | wc -l
> 951
^ permalink raw reply
* Re: [PATCH v7 2/2] drivers: soc: Add LLCC driver
From: rishabhb @ 2018-05-23 17:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-arm Mailing List, linux-arm-msm, devicetree,
Linux Kernel Mailing List, linux-arm, tsoni, ckadabi, Evan Green,
Rob Herring
In-Reply-To: <CAHp75Vd8HZU+BT38-OfXHiihv1yZG6YBeMWyfweBA+kAwk6HUw@mail.gmail.com>
On 2018-05-22 12:38, Andy Shevchenko wrote:
> On Tue, May 22, 2018 at 9:33 PM, <rishabhb@codeaurora.org> wrote:
>> On 2018-05-18 14:01, Andy Shevchenko wrote:
>>> On Wed, May 16, 2018 at 8:43 PM, Rishabh Bhatnagar
>>> <rishabhb@codeaurora.org> wrote:
>
>>>> +#define ACTIVATE 0x1
>>>> +#define DEACTIVATE 0x2
>>>> +#define ACT_CTRL_OPCODE_ACTIVATE 0x1
>>>> +#define ACT_CTRL_OPCODE_DEACTIVATE 0x2
>>>> +#define ACT_CTRL_ACT_TRIG 0x1
>>>
>>>
>>> Are these bits? Perhaps BIT() ?
>>>
>> isn't it just better to use fixed size as u suggest in the next
>> comment?
>
> If the are bits, use BIT() macro.
>
>>>> +struct llcc_slice_desc *llcc_slice_getd(u32 uid)
>>>> +{
>>>> + const struct llcc_slice_config *cfg;
>>>> + struct llcc_slice_desc *desc;
>>>> + u32 sz, count = 0;
>>>> +
>>>> + cfg = drv_data->cfg;
>>>> + sz = drv_data->cfg_size;
>>>> +
>>>
>>>
>>>> + while (cfg && count < sz) {
>>>> + if (cfg->usecase_id == uid)
>>>> + break;
>>>> + cfg++;
>>>> + count++;
>>>> + }
>>>> + if (cfg == NULL || count == sz)
>>>> + return ERR_PTR(-ENODEV);
>>>
>>>
>>> if (!cfg)
>>> return ERR_PTR(-ENODEV);
>>>
>>> while (cfg->... != uid) {
>>> cfg++;
>>> count++;
>>> }
>>>
>>> if (count == sz)
>>> return ...
>>>
>>> Though I would rather put it to for () loop.
>>>
>> In each while loop iteration the cfg pointer needs to be checked for
>> NULL. What if the usecase id never matches the uid passed by client
>> and we keep iterating. At some point it will crash.
>
> do {
> if (!cfg || count == sz)
> return ...(-ENODEV);
> ...
> } while (...);
>
> Though, as I said for-loop will look slightly better I think.
>
>>>> + ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
>>>> + DEACTIVATE);
>>>
>>>
>>> Perhaps one line (~83 characters here is OK) ?
>>
>> The checkpatch script complains about such lines.
>
> So what if it just 3 characters out?
>
Many upstream reviewers have objection to lines crossing over 80
characters
I have gotten reviews to reduce the line length even if its like 81~82
characters. Can we keep this as it is? I have addressed all other
comments and will send out the next patch by today.
>>>> + ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
>>>> + ACTIVATE);
>
>>> Ditto.
>
>>>> + attr1_cfg = bcast_off +
>>>> +
>>>> LLCC_TRP_ATTR1_CFGn(llcc_table[i].slice_id);
>>>> + attr0_cfg = bcast_off +
>>>> +
>>>> LLCC_TRP_ATTR0_CFGn(llcc_table[i].slice_id);
>
>>> Ditto.
>
>>>> + attr1_val |= llcc_table[i].probe_target_ways <<
>>>> + ATTR1_PROBE_TARGET_WAYS_SHIFT;
>>>> + attr1_val |= llcc_table[i].fixed_size <<
>>>> + ATTR1_FIXED_SIZE_SHIFT;
>>>> + attr1_val |= llcc_table[i].priority <<
>>>> ATTR1_PRIORITY_SHIFT;
>
>>> foo |=
>>> bar << SHIFT;
>>>
>>> would look slightly better.
>
> Did you consider this option ?
^ permalink raw reply
* RE: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver
From: ilialin @ 2018-05-23 17:42 UTC (permalink / raw)
To: 'Sudeep Holla', vireshk, nm, sboyd, robh, mark.rutland,
rjw, linux-pm, devicetree, linux-kernel
In-Reply-To: <1ec7645d-72b6-5a1a-48c3-831a3c484a0e@arm.com>
> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: Wednesday, May 23, 2018 16:25
> To: Ilia Lin <ilialin@codeaurora.org>; vireshk@kernel.org; nm@ti.com;
> sboyd@kernel.org; robh@kernel.org; mark.rutland@arm.com;
> rjw@rjwysocki.net; linux-pm@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Subject: Re: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver
>
>
>
> On 23/05/18 13:38, Ilia Lin wrote:
> > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO
> > processors, the CPU frequency subset and voltage value of each OPP
> > varies based on the silicon variant in use. Qualcomm Process Voltage
> > Scaling Tables defines the voltage and frequency value based on the
> > msm-id in SMEM and speedbin blown in the efuse combination.
> > The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the
> > SoC to provide the OPP framework with required information.
> > This is used to determine the voltage and frequency value for each OPP
> > of
> > operating-points-v2 table when it is parsed by the OPP framework.
> >
> > Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
> > ---
> > drivers/cpufreq/Kconfig.arm | 10 ++
> > drivers/cpufreq/Makefile | 1 +
> > drivers/cpufreq/cpufreq-dt-platdev.c | 3 +
> > drivers/cpufreq/qcom-cpufreq-kryo.c | 181
> > +++++++++++++++++++++++++++++++++++
> > 4 files changed, 195 insertions(+)
> > create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
> >
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index de55c7d..0bfd40e 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
> > depends on ARCH_OMAP2PLUS
> > default ARCH_OMAP2PLUS
> >
> > +config ARM_QCOM_CPUFREQ_KRYO
> > + bool "Qualcomm Kryo based CPUFreq"
> > + depends on QCOM_QFPROM
> > + depends on QCOM_SMEM
> > + select PM_OPP
> > + help
> > + This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
> > +
> > + If in doubt, say N.
> > +
>
> Sorry but just noticed now, any reason why this can't be module. I can't
> imagine any.
I was asked previously to change this from tristate to bool.
>
> [..]
>
> > +static int qcom_cpufreq_kryo_probe(struct platform_device *pdev) {
> > + struct opp_table *opp_tables[NR_CPUS] = {0};
> > + struct platform_device *cpufreq_dt_pdev;
> > + enum _msm8996_version msm8996_version;
> > + struct nvmem_cell *speedbin_nvmem;
> > + struct device_node *np;
> > + struct device *cpu_dev;
>
> [..]
>
> > +
> > + cpufreq_dt_pdev = platform_device_register_simple("cpufreq-dt", -
> 1, NULL, 0);
> > + if (!IS_ERR(cpufreq_dt_pdev))
> > + return 0;
> > +
> > + ret = PTR_ERR(cpufreq_dt_pdev);
> > + dev_err(cpu_dev, "Failed to register platform device\n");
> > +
> > +free_opp:
> > + for_each_possible_cpu(cpu) {
> > + if (IS_ERR_OR_NULL(opp_tables[cpu]))
> > + break;
> > + dev_pm_opp_put_supported_hw(opp_tables[cpu]);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int __init qcom_cpufreq_kryo_init(void) {
> > + /*
> > + * Since the driver depends on smem and nvmem drivers, which may
> > + * return EPROBE_DEFER, all the real activity is done in the probe,
> > + * which may be defered as well. The init here is only registering
> > + * a platform device.
> > + */
> > + platform_device_register_simple("qcom-cpufreq-kryo", -1, NULL, 0);
> > + return 0;
> > +}
> > +module_init(qcom_cpufreq_kryo_init);
>
> Do you need this at all ? See below on how to eliminate the need for this.
>
> > +
> > +static struct platform_driver qcom_cpufreq_kryo_driver = {
> > + .probe = qcom_cpufreq_kryo_probe,
> > + .driver = {
> > + .name = "qcom-cpufreq-kryo",
> > + },
> > +};
> > +builtin_platform_driver(qcom_cpufreq_kryo_driver);
>
> Use builtin_platform_driver_probe and remove qcom_cpufreq_kryo_init or
> use module_platform_driver_probe if it can be module.
>
> --
> Regards,
> Sudeep
^ permalink raw reply
* Re: [PATCH RFC 07/24] drm/lima: add mali 4xx GPU hardware regs
From: Vasily Khoruzhick @ 2018-05-23 17:31 UTC (permalink / raw)
To: Rob Herring
Cc: Simon Shields, devicetree, Connor Abbott, Marek Vasut,
Neil Armstrong, Andrei Paulau, dri-devel, Qiang Yu, Erico Nunes
In-Reply-To: <20180523172401.GC17409@rob-hp-laptop>
On Wed, May 23, 2018 at 10:24 AM, Rob Herring <robh@kernel.org> wrote:
> On Fri, May 18, 2018 at 05:27:58PM +0800, Qiang Yu wrote:
>> From: Lima Project Developers <dri-devel@lists.freedesktop.org>
>>
>> Signed-off-by: Qiang Yu <yuq825@gmail.com>
>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>> ---
>> drivers/gpu/drm/lima/lima_regs.h | 304 +++++++++++++++++++++++++++++++
>> 1 file changed, 304 insertions(+)
>> create mode 100644 drivers/gpu/drm/lima/lima_regs.h
>>
>> diff --git a/drivers/gpu/drm/lima/lima_regs.h b/drivers/gpu/drm/lima/lima_regs.h
>> new file mode 100644
>> index 000000000000..ea4a37d69b98
>> --- /dev/null
>> +++ b/drivers/gpu/drm/lima/lima_regs.h
>> @@ -0,0 +1,304 @@
>> +/*
>> + * Copyright (C) 2010-2017 ARM Limited. All rights reserved.
>
> I assume this came from ARM's out of tree kernel driver source. You
> should document what it was based on.
>
>> + * Copyright (C) 2017-2018 Lima Project
>
> IANAL, but is Lima Project a legal entity that can copyright things?
AFAIR it's not a legal entity, and I believe Qiang can simply replace
"Lima Project" with his name in kernel driver. I don't think that
anyone else
made contribution to the kernel driver that is significant enough to
copyright it.
Luc Verhaegen may hold copyright for REed data structures if there're
any in kernel driver - but I believe only userspace parts took
something from limare.
>> + *
>> + * This program is free software and is provided to you under
>> + * the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation, and any use by
>> + * you of this program is subject to the terms of such GNU
>> + * licence.
>> + *
>> + * A copy of the licence is included with the program, and
>> + * can also be obtained from Free Software Foundation, Inc.,
>> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
>
> You can use SPDX tags instead.
>
> Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH RFC 24/24] drm/lima: add makefile and kconfig
From: Rob Herring @ 2018-05-23 17:26 UTC (permalink / raw)
To: Marek Vasut, Qiang Yu
Cc: Simon Shields, devicetree, Connor Abbott, Neil Armstrong,
Andrei Paulau, dri-devel, Vasily Khoruzhick, Erico Nunes
In-Reply-To: <7e0835f9-a368-294d-9483-91513211cf24@denx.de>
On Wed, May 23, 2018 at 07:16:41PM +0200, Marek Vasut wrote:
> On 05/18/2018 11:28 AM, Qiang Yu wrote:
> > From: Lima Project Developers <dri-devel@lists.freedesktop.org>
> >
> > Signed-off-by: Qiang Yu <yuq825@gmail.com>
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > Signed-off-by: Simon Shields <simon@lineageos.org>
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > drivers/gpu/drm/Kconfig | 2 ++
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/lima/Kconfig | 9 +++++++++
> > drivers/gpu/drm/lima/Makefile | 19 +++++++++++++++++++
> > 4 files changed, 31 insertions(+)
> > create mode 100644 drivers/gpu/drm/lima/Kconfig
> > create mode 100644 drivers/gpu/drm/lima/Makefile
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index deeefa7a1773..f00d529ee034 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -289,6 +289,8 @@ source "drivers/gpu/drm/pl111/Kconfig"
> >
> > source "drivers/gpu/drm/tve200/Kconfig"
> >
> > +source "drivers/gpu/drm/lima/Kconfig"
> > +
> > # Keep legacy drivers last
> >
> > menuconfig DRM_LEGACY
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 50093ff4479b..aba686e41d6b 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -103,3 +103,4 @@ obj-$(CONFIG_DRM_MXSFB) += mxsfb/
> > obj-$(CONFIG_DRM_TINYDRM) += tinydrm/
> > obj-$(CONFIG_DRM_PL111) += pl111/
> > obj-$(CONFIG_DRM_TVE200) += tve200/
> > +obj-$(CONFIG_DRM_LIMA) += lima/
> > diff --git a/drivers/gpu/drm/lima/Kconfig b/drivers/gpu/drm/lima/Kconfig
> > new file mode 100644
> > index 000000000000..4ce9ac2e8204
> > --- /dev/null
> > +++ b/drivers/gpu/drm/lima/Kconfig
> > @@ -0,0 +1,9 @@
> > +
> > +config DRM_LIMA
> > + tristate "LIMA (DRM support for ARM Mali 400/450 GPU)"
> > + depends on DRM
> > + depends on ARCH_SUNXI || ARCH_ROCKCHIP || ARCH_EXYNOS || ARCH_MESON
>
> You can add ARCH_ZYNQMP here too , it has Mali 400 MP2.
Better yet, just drop them all rather than continually adding to the
list.
But if you keep it, add '|| COMPILE_TEST'.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox