* Re: [PATCH] net: add init-regs for of_phy support
From: Ben Dooks @ 2014-02-17 13:53 UTC (permalink / raw)
To: Mark Rutland
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-sh@vger.kernel.org
In-Reply-To: <20140217134448.GA19308@e106331-lin.cambridge.arm.com>
On 17/02/14 13:44, Mark Rutland wrote:
> On Mon, Feb 17, 2014 at 01:08:04PM +0000, Ben Dooks wrote:
>> Add new init-regs field for of_phy nodes and make sure these
>> get applied when the phy is configured.
>>
>> This allows any phy node in an fdt to initialise registers
>> that may not be set as standard by the driver at initialisation
>> time, such as LED controls.
>
> Why not have a driver for the particular PHY? If it's not standard we
> don't need to pretend it is. If it has some extensions then the standard
> compatible string can be a fallback entry in the compatible list.
>
> I think allocating a compatible string and handling it in the kernel is
> better than having arbitrary register poke values in the dt.
>> + prop = of_find_property(of_node, "init-regs", &len);
>> + if (prop) {
>> + if (len % (sizeof(__be32) * 3)) {
>> + dev_err(dev, "init-regs not multiple of 3 entries\n");
>> + return -EINVAL;
>> + }
>> +
>> + ptr = of_prop_next_u32(prop, ptr, ®);
>> + while (ptr != NULL) {
>> + ptr = of_prop_next_u32(prop, ptr, ®);
oops, bad rebase fixup, this should have been deleted.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Ben Dooks @ 2014-02-17 13:50 UTC (permalink / raw)
To: Mark Rutland
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-sh@vger.kernel.org
In-Reply-To: <20140217134448.GA19308@e106331-lin.cambridge.arm.com>
On 17/02/14 13:44, Mark Rutland wrote:
> On Mon, Feb 17, 2014 at 01:08:04PM +0000, Ben Dooks wrote:
>> Add new init-regs field for of_phy nodes and make sure these
>> get applied when the phy is configured.
>>
>> This allows any phy node in an fdt to initialise registers
>> that may not be set as standard by the driver at initialisation
>> time, such as LED controls.
>
> Why not have a driver for the particular PHY? If it's not standard we
> don't need to pretend it is. If it has some extensions then the standard
> compatible string can be a fallback entry in the compatible list.
I was trying to provide some useful and reasonably generic code
to setup PHYs without having to add specific arguments to each of
them.
I could have added something like ksz8041,led-mode1 = <1> and
updated the micrel driver.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Mark Rutland @ 2014-02-17 13:44 UTC (permalink / raw)
To: Ben Dooks
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-sh@vger.kernel.org
In-Reply-To: <1392642484-19938-2-git-send-email-ben.dooks@codethink.co.uk>
On Mon, Feb 17, 2014 at 01:08:04PM +0000, Ben Dooks wrote:
> Add new init-regs field for of_phy nodes and make sure these
> get applied when the phy is configured.
>
> This allows any phy node in an fdt to initialise registers
> that may not be set as standard by the driver at initialisation
> time, such as LED controls.
Why not have a driver for the particular PHY? If it's not standard we
don't need to pretend it is. If it has some extensions then the standard
compatible string can be a fallback entry in the compatible list.
I think allocating a compatible string and handling it in the kernel is
better than having arbitrary register poke values in the dt.
Thanks,
Mark.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
> drivers/net/phy/phy_device.c | 59 ++++++++++++++++++++++++++-
> 2 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index 58307d0..48d8ded 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -20,6 +20,8 @@ Optional Properties:
> assume clause 22. The compatible list may also contain other
> elements.
> - max-speed: Maximum PHY supported speed (10, 100, 1000...)
> +- init-regs: Set of registers to modify at initialisation as a
> + a set of <register set clear>
>
> Example:
>
> @@ -29,3 +31,13 @@ ethernet-phy@0 {
> interrupts = <35 1>;
> reg = <0>;
> };
> +
> +ethernet-phy@0 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + interrupt-parent = <40000>;
> + interrupts = <35 1>;
> + reg = <0>;
> +
> + /* set KSZ8041 LED mode bits correctly */
> + init-reg = <0x1e 0x4000 0xc000>;
> +};
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 82514e7..6741cdb 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -33,6 +33,7 @@
> #include <linux/mdio.h>
> #include <linux/io.h>
> #include <linux/uaccess.h>
> +#include <linux/of.h>
>
> #include <asm/irq.h>
>
> @@ -532,6 +533,57 @@ static int phy_poll_reset(struct phy_device *phydev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static int of_phy_configure(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->dev;
> + struct device_node *of_node = dev->of_node;
> + struct property *prop;
> + const __be32 *ptr;
> + u32 reg, set, clear;
> + int len;
> + int val;
> +
> + if (!of_node)
> + of_node = dev->parent->of_node;
> + if (!of_node)
> + return 0;
> +
> + prop = of_find_property(of_node, "init-regs", &len);
> + if (prop) {
> + if (len % (sizeof(__be32) * 3)) {
> + dev_err(dev, "init-regs not multiple of 3 entries\n");
> + return -EINVAL;
> + }
> +
> + ptr = of_prop_next_u32(prop, ptr, ®);
> + while (ptr != NULL) {
> + ptr = of_prop_next_u32(prop, ptr, ®);
> + ptr = of_prop_next_u32(prop, ptr, &set);
> + ptr = of_prop_next_u32(prop, ptr, &clear);
> +
> + val = phy_read(phydev, reg);
> + if (val < 0) {
> + dev_err(dev, "failed to read %d\n", reg);
> + return val;
> + }
> +
> + val &= ~clear;
> + val |= set;
> + phy_write(phydev, reg, val);
> +
> + dev_info(dev, "set d to %04x\n", reg, val);
> +
> + ptr = of_prop_next_u32(prop, ptr, ®);
> + }
> + }
> +
> + return 0;
> +}
> +#else
> +static inline int of_phy_configure(struct phy_device *phydev) { return 0; }
> +#endif
> +
> int phy_init_hw(struct phy_device *phydev)
> {
> int ret;
> @@ -551,7 +603,12 @@ int phy_init_hw(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> - return phydev->drv->config_init(phydev);
> + ret = phydev->drv->config_init(phydev);
> +
> + if (ret == 0)
> + ret = of_phy_configure(phydev);
> +
> + return ret;
> }
> EXPORT_SYMBOL(phy_init_hw);
>
> --
> 1.8.5.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
* [PATCH] net: add init-regs for of_phy support
From: Ben Dooks @ 2014-02-17 13:08 UTC (permalink / raw)
To: netdev, devicetree; +Cc: linux-sh, Ben Dooks
In-Reply-To: <1392642484-19938-1-git-send-email-ben.dooks@codethink.co.uk>
Add new init-regs field for of_phy nodes and make sure these
get applied when the phy is configured.
This allows any phy node in an fdt to initialise registers
that may not be set as standard by the driver at initialisation
time, such as LED controls.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
drivers/net/phy/phy_device.c | 59 ++++++++++++++++++++++++++-
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 58307d0..48d8ded 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -20,6 +20,8 @@ Optional Properties:
assume clause 22. The compatible list may also contain other
elements.
- max-speed: Maximum PHY supported speed (10, 100, 1000...)
+- init-regs: Set of registers to modify at initialisation as a
+ a set of <register set clear>
Example:
@@ -29,3 +31,13 @@ ethernet-phy@0 {
interrupts = <35 1>;
reg = <0>;
};
+
+ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ interrupt-parent = <40000>;
+ interrupts = <35 1>;
+ reg = <0>;
+
+ /* set KSZ8041 LED mode bits correctly */
+ init-reg = <0x1e 0x4000 0xc000>;
+};
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 82514e7..6741cdb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -33,6 +33,7 @@
#include <linux/mdio.h>
#include <linux/io.h>
#include <linux/uaccess.h>
+#include <linux/of.h>
#include <asm/irq.h>
@@ -532,6 +533,57 @@ static int phy_poll_reset(struct phy_device *phydev)
return 0;
}
+#ifdef CONFIG_OF
+static int of_phy_configure(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+ struct property *prop;
+ const __be32 *ptr;
+ u32 reg, set, clear;
+ int len;
+ int val;
+
+ if (!of_node)
+ of_node = dev->parent->of_node;
+ if (!of_node)
+ return 0;
+
+ prop = of_find_property(of_node, "init-regs", &len);
+ if (prop) {
+ if (len % (sizeof(__be32) * 3)) {
+ dev_err(dev, "init-regs not multiple of 3 entries\n");
+ return -EINVAL;
+ }
+
+ ptr = of_prop_next_u32(prop, ptr, ®);
+ while (ptr != NULL) {
+ ptr = of_prop_next_u32(prop, ptr, ®);
+ ptr = of_prop_next_u32(prop, ptr, &set);
+ ptr = of_prop_next_u32(prop, ptr, &clear);
+
+ val = phy_read(phydev, reg);
+ if (val < 0) {
+ dev_err(dev, "failed to read %d\n", reg);
+ return val;
+ }
+
+ val &= ~clear;
+ val |= set;
+ phy_write(phydev, reg, val);
+
+ dev_info(dev, "set d to %04x\n", reg, val);
+
+ ptr = of_prop_next_u32(prop, ptr, ®);
+ }
+ }
+
+ return 0;
+}
+#else
+static inline int of_phy_configure(struct phy_device *phydev) { return 0; }
+#endif
+
int phy_init_hw(struct phy_device *phydev)
{
int ret;
@@ -551,7 +603,12 @@ int phy_init_hw(struct phy_device *phydev)
if (ret < 0)
return ret;
- return phydev->drv->config_init(phydev);
+ ret = phydev->drv->config_init(phydev);
+
+ if (ret == 0)
+ ret = of_phy_configure(phydev);
+
+ return ret;
}
EXPORT_SYMBOL(phy_init_hw);
--
1.8.5.3
^ permalink raw reply related
* RFC: add init-regs for phy nodes
From: Ben Dooks @ 2014-02-17 13:08 UTC (permalink / raw)
To: netdev, devicetree; +Cc: linux-sh
I have run into an issue where the KSZ8041 does not get
initialised properly on the Renesas R8A7790 lager board.
This patch adds the ability to set arbitrary registers
on initialisation of a phy from informaiton in the fdt.
Feedback welcome.
^ permalink raw reply
* Re: [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Arnd Bergmann @ 2014-02-17 13:00 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Tomasz Figa, linux-arm-kernel, mark.rutland, devicetree,
Ulf Hansson, Pawel Moll, Ian Campbell, Tomasz Figa, linux-mmc,
Chris Ball, robh+dt, Kumar Gala, Olof Johansson, Fabio Estevam,
Sascha Hauer
In-Reply-To: <20140215205159.GL30257@n2100.arm.linux.org.uk>
On Saturday 15 February 2014, Russell King - ARM Linux wrote:
> On Sat, Feb 15, 2014 at 05:21:11PM +0100, Arnd Bergmann wrote:
> > On Saturday 15 February 2014 14:22:30 Tomasz Figa wrote:
> > > On 15.02.2014 14:09, Arnd Bergmann wrote:
> > >
> > > > For spi-mode SDIO devices I'm assuming it's similar, except that
> > > > you'd describe the actual SDIO device in the board info rather than
> > > > create a fake SDIO controller. Still not discoverable unless I'm
> > > > missing your point.
> > >
> > > I'm not sure if we should assume that SPI = MMC over SPI. I believe
> > > there might be a custom protocol involved as well.
> >
> > In case of SD/MMC, you essentially have three separate command sets:
> > SPI, MMC and SD, and each of them has multiple versions. MMC and SD
> > compatible devices generally also support the SPI command set (IIRC
> > it is required,
>
> SPI support is mandatory for SDIO as well.
>
> SDIO has CIS (remember card information structures... like PCMCIA)
> which identifies the various different logical functions of the device,
> giving class information, vendor information etc.
>
> So... certainly the type of the attached device is discoverable even
> on SPI
But how are SPI SDIO devices used in Linux and described in DT?
If we always use the MMC_SPI host controller driver and only
describe the controller in DT, then you are right, and there is
no difference to the normal SDIO case.
If we just describe it as an SPI device OTOH, the fact that we can
identify it is useless, because we first have to know what it
is before we can ask for its identity.
> > If a device supports both SDIO and SPI, I think a straightforward
> > implementation would be to use the exact same command set, but
> > you are right that this isn't the only possibility, and the SD/MMC
> > shows how they can be slightly different already.
>
> Given that the SPI mode is mandatory for SDIO cards, why would you
> also implement another SPI mode with different commands?
I don't know why, I was just saying that it's what storage SD cards do.
> > > Stepping aside from SPI, I already gave an example of a WLAN chip that
> > > supports multiple control busses [1]. In addition to the commonly used
> > > SDIO, it supports USB and HSIC as well:
> > >
> > > [1] http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
> > >
> > > Moreover, some of Samsung boards use HSIC to communicate with modem
> > > chips, which have exactly the same problem as we're trying to solve here
> > > - they need to be powered on to be discovered.
> >
> > Thanks, this definitely makes a good example. I see that it also
> > supports SPI mode for SDIO as mentioned in your link.
>
> Well, USB is another discoverable bus. As HSIC is a derivative of USB,
> I'd be surprised if it weren't discoverable there too.
>
> So, out of everything identified so far, we have no undiscoverable buses.
The device also lists a high-speed uart interface. While in Linux a
tty line discipline is not exactly the same as a driver, I would
still count it as a nondiscoverable interface.
> - requires different DT if the chip is changed, which causes problems
> for users to identify which out of zillions of DT files they should
> use for their exact platform.
I'm still not buying this argument. Making any changes to the board
design that are visible to software can have this effect.
> - have to work out how to match up the fake device with a probed device
> when it becomes available: existing SDIO drivers all assume that the
> card has been through a fairly complex initialisation sequence already.
This one is an implementation detail. Yes, we have to ensure it works,
but it's not inherently harder that getting the other approach to work.
> - multi-function SDIO is much harder to deal with since you have mutliple
> drivers involved, and the SDIO device as a whole needs initialisation
> before anyone can drive it.
Agreed, this point is a big one that I hadn't considered before.
I agree that my approach isn't ideal for multifunction cards, especially
when you want to be able to load function drivers individually or have
only one of them in use. It can probably work fine for things like calling
clk_get() where you can simply describe the clock in each function device
node separately, but I don't see how I'd easily use a 'reset' line
that has to be triggered for the first function driver that gets loaded
but must not be triggered again for the second one :(
> - adds complexity to the SDIO drivers; they would have to know whether
> they're embedded or on a plug-in card.
How so? If a device can be a plug-in card, I would assume that it is
actually discoverable without a special power-on sequence, while a device
that needs this can't be used standalone.
Arnd
^ permalink raw reply
* [PATCH 2/3] ARM: DT: STi: Add DT node for ST's SATA device
From: Lee Jones @ 2014-02-17 12:56 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: alexandre.torgue, Lee Jones, devicetree, Srinivas Kandagatla
In-Reply-To: <1392641818-23419-1-git-send-email-lee.jones@linaro.org>
Cc: devicetree@vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
arch/arm/boot/dts/stih416-b2020-revE.dts | 4 ++++
arch/arm/boot/dts/stih416-b2020.dts | 4 ++++
arch/arm/boot/dts/stih416.dtsi | 16 ++++++++++++++++
3 files changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
index 693d0ec..7350a86 100644
--- a/arch/arm/boot/dts/stih416-b2020-revE.dts
+++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
@@ -37,5 +37,9 @@
st,pcie-tx-pol-inv;
st,sata-gen = <3>;
};
+
+ sata0: sata@fe380000{
+ status = "okay";
+ };
};
};
diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
index fd9cbad..ebd784b 100644
--- a/arch/arm/boot/dts/stih416-b2020.dts
+++ b/arch/arm/boot/dts/stih416-b2020.dts
@@ -18,5 +18,9 @@
st,pcie_tx_pol_inv = <1>;
st,sata_gen = "gen3";
};
+
+ sata0: sata@fe380000{
+ status = "okay";
+ };
};
};
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 9fd8efb..d1c874f 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -153,5 +153,21 @@
#phy-cells = <2>;
st,syscfg = <&syscfg_rear>;
};
+
+ sata0: sata@fe380000 {
+ compatible = "st,ahci";
+ reg = <0xfe380000 0x1000>;
+ interrupts = <GIC_SPI 157 IRQ_TYPE_NONE>;
+ interrupt-names = "hostc";
+ phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+ phy-names = "ahci_phy";
+ resets = <&powerdown STIH416_SATA0_POWERDOWN>,
+ <&softreset STIH416_SATA0_SOFTRESET>;
+ reset-names = "pwr-dwn", "sw-rst";
+ clock-names = "ahci_clk";
+ clocks = <&CLK_S_ICN_REG_0>;
+
+ status = "disabled";
+ };
};
};
--
1.8.3.2
^ permalink raw reply related
* [PATCH 1/3] ahci: st: Provide DT bindings for ST's SATA implementation
From: Lee Jones @ 2014-02-17 12:56 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: alexandre.torgue, Lee Jones, devicetree, Srinivas Kandagatla
Cc: devicetree@vger.kernel.org
Cc: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
Documentation/devicetree/bindings/ata/ahci-st.txt | 31 +++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ata/ahci-st.txt
diff --git a/Documentation/devicetree/bindings/ata/ahci-st.txt b/Documentation/devicetree/bindings/ata/ahci-st.txt
new file mode 100644
index 0000000..1b69fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-st.txt
@@ -0,0 +1,31 @@
+STMicroelectronics STi SATA controller
+
+This binding describes a SATA device.
+
+Required properties:
+ - compatible : Must be "st,ahci"
+ - reg : Physical base addresses and length of register sets
+ - interrupts : Interrupt associated with the SATA device
+ - interrupt-names : Associated name must be; "hostc"
+ - resets : The power-down and soft-reset lines of SATA IP
+ - reset-names : Associated names must be; "pwr-dwn" and "sw-rst"
+ - clocks : The phandle for the clock
+ - clock-names : Associated name must be; "ahci_clk"
+ - phys : The phandle for the PHY device
+ - phy-names : Associated name must be; "ahci_phy"
+
+Example:
+
+ sata0: sata@fe380000 {
+ compatible = "st,ahci";
+ reg = <0xfe380000 0x1000>;
+ interrupts = <GIC_SPI 157 IRQ_TYPE_NONE>;
+ interrupt-names = "hostc";
+ phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+ phy-names = "ahci_phy";
+ resets = <&powerdown STIH416_SATA0_POWERDOWN>,
+ <&softreset STIH416_SATA0_SOFTRESET>;
+ reset-names = "pwr-dwn", "sw-rst";
+ clocks = <&CLK_S_ICN_REG_0>;
+ clock-names = "ahci_clk";
+ };
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH V2 2/2] ARM: shmobile: r8a7791: add i2c2 bus to koelsch dt
From: Magnus Damm @ 2014-02-17 12:07 UTC (permalink / raw)
To: Wolfram Sang
Cc: SH-Linux, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, Simon Horman, Wolfram Sang
In-Reply-To: <1392633882-12142-2-git-send-email-wsa@the-dreams.de>
On Mon, Feb 17, 2014 at 7:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
> ---
> arch/arm/boot/dts/r8a7791-koelsch.dts | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
Acked-by: Magnus Damm <damm@opensource.se>
Thanks,
/ magnus
^ permalink raw reply
* Re: [PATCH V2 1/2] ARM: shmobile: r8a7791: add i2c master nodes to dtsi
From: Magnus Damm @ 2014-02-17 12:06 UTC (permalink / raw)
To: Wolfram Sang
Cc: SH-Linux, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, Simon Horman, Wolfram Sang
In-Reply-To: <1392633882-12142-1-git-send-email-wsa@the-dreams.de>
On Mon, Feb 17, 2014 at 7:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
> ---
> arch/arm/boot/dts/r8a7791.dtsi | 69 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index 41194fe..1ab4f3d 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -19,6 +19,15 @@
> #address-cells = <2>;
> #size-cells = <2>;
>
> + aliases {
> + i2c0 = &i2c0;
> + i2c1 = &i2c1;
> + i2c2 = &i2c2;
> + i2c3 = &i2c3;
> + i2c4 = &i2c4;
> + i2c5 = &i2c5;
> + };
> +
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -170,6 +179,66 @@
> <0 17 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> + i2c0: i2c@e6508000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "renesas,i2c-r8a7791";
V2 of this patch looks good to me, thanks!
Acked-by: Magnus Damm <damm@opensource.se>
When you have time, please send an incremental patch to add support
for the three IIC channels as well.
Cheers,
/ magnus
^ permalink raw reply
* Re: [PATCH v3 1/7] Documentation: Add device tree bindings for Freescale i.MX GPC
From: Philipp Zabel @ 2014-02-17 11:08 UTC (permalink / raw)
To: Mark Rutland
Cc: Shawn Guo, Rob Herring,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20140217104936.GB18920-NuALmloUBlrZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
Hi Mark,
thank you for the comments.
Am Montag, den 17.02.2014, 10:49 +0000 schrieb Mark Rutland:
> On Mon, Feb 17, 2014 at 10:04:57AM +0000, Philipp Zabel wrote:
> > The i.MX6 contains a power controller that controls power gating and
> > sequencing for the SoC's power domains.
> >
> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > .../devicetree/bindings/power/fsl,imx-gpc.txt | 61 ++++++++++++++++++++++
> > 1 file changed, 61 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > new file mode 100644
> > index 0000000..3ec8c0e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > @@ -0,0 +1,61 @@
> > +Freescale i.MX General Power Controller
> > +=======================================
> > +
> > +The i.MX6Q General Power Control (GPC) block contains DVFS load tracking
> > +counters and Power Gating Control (PGC) for the CPU and PU (GPU/VPU) power
> > +domains.
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,imx6q-gpc"
> > +- reg: should be register base and length as documented in the
> > + datasheet
> > +- interrupts: Should contain GPC interrupt request 1
>
> Does the unit have multiple interrupts?
According to the i.MX6 Reference Manuals, interrupt 121 is the "GPC
interrupt request 1". The following interrupt 122 (which also is listed
in the current imx6qdl.dtsi) is "Reserved". I think the answer is no,
maybe Shawn can correct me.
> If so it would be good to use interrupt-names so they can be describe
> unambiguously.
>
> > +- pu-supply: Link to the LDO regulator powering the PU power domain
> > +- #address-cells, #size-cells: Should be <1>
>
> This seems to be to map child nodes into the same address space. Is
> there any need that these are precisely 1, or could they be anything
> that maps the child nodes in?
I guess there is not. Should I just drop this line?
> > +
> > +The gpc node should contain 'power-domain' subnodes for each power domain.
> > +These serve as phandle targets for devices belonging to the power domain:
> > +
> > +Power domains controlled by a PGC register set
> > +==============================================
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,imx6q-power-domain"
> > +- reg: should be register base and length as documented in the
> > + datasheet
> > +
> > +Specifying power domain for IP modules
> > +======================================
> > +
> > +IP cores belonging to a power domain should contain a 'power-domain' property
> > +that is a phandle pointing to the power-domain subnode of the gpc device node.
> > +
> > +Required properties:
> > +- power-domain: A phandle pointing to the power-domain device tree node
>
> This sounds a little generic. Is there a standard power-domain binding?
> If not it might be better for the moment for this to be
> fsl,power-domain.
So far I am only aware of samsung,power-domain, so I'll change this to
fsl,power-domain accordingly.
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4] bus: imx-weim: support CS GPR configuration
From: Philippe De Muyter @ 2014-02-17 10:52 UTC (permalink / raw)
To: Shawn Guo
Cc: Alexander Shiyan,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Huang Shijie,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, Mark Rutland
In-Reply-To: <20140217092243.GB10731-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
On Mon, Feb 17, 2014 at 05:22:47PM +0800, Shawn Guo wrote:
> On Mon, Feb 17, 2014 at 09:52:21AM +0100, Philippe De Muyter wrote:
> > On Sun, Feb 16, 2014 at 10:31:16PM +0800, Shawn Guo wrote:
> > > On Sun, Feb 16, 2014 at 06:22:44PM +0400, Alexander Shiyan wrote:
> > > > Воскресенье, 16 февраля 2014, 22:03 +08:00 от Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> > > > > For imx50-weim and imx6q-weim type of devices, there might a WEIM CS
> > > > > space configuration register in General Purpose Register controller,
> > > > > e.g. IOMUXC_GPR1 on i.MX6Q.
> > > > ...
> > > > > +static int __init imx_weim_gpr_setup(struct platform_device *pdev)
> > > > > +{
> > > > > + struct device_node *np = pdev->dev.of_node;
> > > > > + struct property *prop;
> > > > > + const __be32 *p;
> > > > > + struct regmap *gpr;
> > > > > + u32 gprvals[4] = {
> > > > > + 05, /* CS0(128M) CS1(0M) CS2(0M) CS3(0M) */
> > > > > + 033, /* CS0(64M) CS1(64M) CS2(0M) CS3(0M) */
> > > > > + 0113, /* CS0(64M) CS1(32M) CS2(32M) CS3(0M) */
> > > > > + 01111, /* CS0(64M) CS1(32M) CS2(32M) CS3(0M) */
> > > > > + };
> > > > > + u32 gprval = 0;
> > > > > + u32 val;
> > > > > + int cs = 0;
> > > > > + int i = 0;
> > > > > +
> > > > > + gpr = syscon_regmap_lookup_by_phandle(np, "fsl,weim-cs-gpr");
> > > > > + if (IS_ERR(gpr)) {
> > > > > + dev_dbg(&pdev->dev, "failed to find weim-cs-gpr\n");
> > > > > + return 0;
> > > >
> > > > Only one comment:
> > > > You do not use these error codes in the probe(),
> > > > so let's declare this function as void.
> > >
> > > Oh, yes. I should check return of imx_weim_gpr_setup() in probe().
> > > Will add in the next version.
> >
> > Well, as all the error cases are already covered by error messages, there is
> > no added value in testing that again in probe(). As Alexander wrote, you can
> > declare imx_weim_gpr_setup() as 'void'.
>
> Hmm, shouldn't we stop probing when running into the error of invalid
> ranges configuration?
I agree, we could do that :)
Philippe
--
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
--
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/7] Documentation: Add device tree bindings for Freescale i.MX GPC
From: Mark Rutland @ 2014-02-17 10:49 UTC (permalink / raw)
To: Philipp Zabel
Cc: Shawn Guo, Rob Herring,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1392631503-17283-2-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Mon, Feb 17, 2014 at 10:04:57AM +0000, Philipp Zabel wrote:
> The i.MX6 contains a power controller that controls power gating and
> sequencing for the SoC's power domains.
>
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> .../devicetree/bindings/power/fsl,imx-gpc.txt | 61 ++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
>
> diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> new file mode 100644
> index 0000000..3ec8c0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> @@ -0,0 +1,61 @@
> +Freescale i.MX General Power Controller
> +=======================================
> +
> +The i.MX6Q General Power Control (GPC) block contains DVFS load tracking
> +counters and Power Gating Control (PGC) for the CPU and PU (GPU/VPU) power
> +domains.
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6q-gpc"
> +- reg: should be register base and length as documented in the
> + datasheet
> +- interrupts: Should contain GPC interrupt request 1
Does the unit have multiple interrupts?
If so it would be good to use interrupt-names so they can be describe
unambiguously.
> +- pu-supply: Link to the LDO regulator powering the PU power domain
> +- #address-cells, #size-cells: Should be <1>
This seems to be to map child nodes into the same address space. Is
there any need that these are precisely 1, or could they be anything
that maps the child nodes in?
> +
> +The gpc node should contain 'power-domain' subnodes for each power domain.
> +These serve as phandle targets for devices belonging to the power domain:
> +
> +Power domains controlled by a PGC register set
> +==============================================
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6q-power-domain"
> +- reg: should be register base and length as documented in the
> + datasheet
> +
> +Specifying power domain for IP modules
> +======================================
> +
> +IP cores belonging to a power domain should contain a 'power-domain' property
> +that is a phandle pointing to the power-domain subnode of the gpc device node.
> +
> +Required properties:
> +- power-domain: A phandle pointing to the power-domain device tree node
This sounds a little generic. Is there a standard power-domain binding?
If not it might be better for the moment for this to be
fsl,power-domain.
Cheers,
Mark.
> +
> +
> +Example:
> +
> + gpc: gpc@020dc000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,imx6q-gpc";
> + reg = <0x020dc000 0x4000>;
> + interrupts = <0 89 0x04 0 90 0x04>;
> + pu-supply = <®_pu>;
> +
> + pd_pu: power-domain@020dc260 {
> + compatible = "fsl,imx6q-power-domain";
> + reg = <0x020dc260 0x10>;
> + };
> + };
> +
> +Example of a device that is part of a power domain:
> +
> + vpu: vpu@02040000 {
> + reg = <0x02040000 0x3c000>;
> + /* ... */
> + power-domain = <&pd_pu>;
> + /* ... */
> + };
> +
> --
> 1.8.5.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V2 2/2] ARM: shmobile: r8a7791: add i2c2 bus to koelsch dt
From: Wolfram Sang @ 2014-02-17 10:44 UTC (permalink / raw)
To: linux-sh
Cc: linux-arm-kernel, devicetree, Magnus Damm, Simon Horman,
Wolfram Sang
In-Reply-To: <1392633882-12142-1-git-send-email-wsa@the-dreams.de>
From: Wolfram Sang <wsa@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
---
arch/arm/boot/dts/r8a7791-koelsch.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index d4b9bba..b3f75d7 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -108,10 +108,29 @@
clock-frequency = <20000000>;
};
+&i2c2 {
+ pinctrl-0 = <&i2c2_pins>;
+ pinctrl-names = "default";
+
+ status = "okay";
+ clock-frequency = <400000>;
+
+ eeprom@50 {
+ compatible = "renesas,24c02";
+ reg = <0x50>;
+ pagesize = <16>;
+ };
+};
+
&pfc {
pinctrl-0 = <&scif0_pins &scif1_pins>;
pinctrl-names = "default";
+ i2c2_pins: i2c {
+ renesas,groups = "i2c2";
+ renesas,function = "i2c2";
+ };
+
scif0_pins: serial0 {
renesas,groups = "scif0_data_d";
renesas,function = "scif0";
--
1.8.5.1
^ permalink raw reply related
* [PATCH V2 1/2] ARM: shmobile: r8a7791: add i2c master nodes to dtsi
From: Wolfram Sang @ 2014-02-17 10:44 UTC (permalink / raw)
To: linux-sh
Cc: linux-arm-kernel, devicetree, Magnus Damm, Simon Horman,
Wolfram Sang
From: Wolfram Sang <wsa@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
---
arch/arm/boot/dts/r8a7791.dtsi | 69 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 41194fe..1ab4f3d 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -19,6 +19,15 @@
#address-cells = <2>;
#size-cells = <2>;
+ aliases {
+ i2c0 = &i2c0;
+ i2c1 = &i2c1;
+ i2c2 = &i2c2;
+ i2c3 = &i2c3;
+ i2c4 = &i2c4;
+ i2c5 = &i2c5;
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -170,6 +179,66 @@
<0 17 IRQ_TYPE_LEVEL_HIGH>;
};
+ i2c0: i2c@e6508000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "renesas,i2c-r8a7791";
+ reg = <0 0xe6508000 0 0x40>;
+ interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
+ status = "disabled";
+ };
+
+ i2c1: i2c@e6518000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "renesas,i2c-r8a7791";
+ reg = <0 0xe6518000 0 0x40>;
+ interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp9_clks R8A7791_CLK_I2C1>;
+ status = "disabled";
+ };
+
+ i2c2: i2c@e6530000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "renesas,i2c-r8a7791";
+ reg = <0 0xe6530000 0 0x40>;
+ interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp9_clks R8A7791_CLK_I2C2>;
+ status = "disabled";
+ };
+
+ i2c3: i2c@e6540000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "renesas,i2c-r8a7791";
+ reg = <0 0xe6540000 0 0x40>;
+ interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp9_clks R8A7791_CLK_I2C3>;
+ status = "disabled";
+ };
+
+ i2c4: i2c@e6520000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "renesas,i2c-r8a7791";
+ reg = <0 0xe6520000 0 0x40>;
+ interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp9_clks R8A7791_CLK_I2C4>;
+ status = "disabled";
+ };
+
+ i2c5: i2c@e6528000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "renesas,i2c-r8a7791";
+ reg = <0 0xe6528000 0 0x40>;
+ interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&mstp9_clks R8A7791_CLK_I2C5>;
+ status = "disabled";
+ };
+
pfc: pfc@e6060000 {
compatible = "renesas,pfc-r8a7791";
reg = <0 0xe6060000 0 0x250>;
--
1.8.5.1
^ permalink raw reply related
* Re: [PATCH RFC v3 3/3] Documentation: arm: define DT idle states bindings
From: Lorenzo Pieralisi @ 2014-02-17 10:40 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, Mike Turquette, Tomasz Figa, Mark Hambleton,
Russell King, Nicolas Pitre, Daniel Lezcano,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
Dave P Martin, Charles Garcia-Tobin, devicetree@vger.kernel.org,
Kevin Hilman, linux-pm@vger.kernel.org, Kumar Gala, Rob Herring,
Vincent Guittot, Antti Miettinen, Peter De Schrijver,
Stephen Boyd, Amit
In-Reply-To: <20140216013956.GV4451@sirena.org.uk>
On Sun, Feb 16, 2014 at 01:39:56AM +0000, Mark Brown wrote:
> On Tue, Feb 11, 2014 at 02:17:53PM +0000, Lorenzo Pieralisi wrote:
>
> > +According to the Server Base System Architecture document (SBSA, [3]), the
> > +power states an ARM CPU can be put into are identified by the following list:
>
> > +1 - Running
> > +2 - Idle_standby
> > +3 - Idle_retention
> > +4 - Sleep
> > +5 - Off
>
> > +ARM platforms implement the power states specified in the SBSA through power
> > +management schemes that allow an OS PM implementation to put the processor in
> > +different idle states (which include states 1 to 4 above; "off" state is not
> > +an idle state since it does not have wake-up capabilities, hence it is not
> > +considered in this document).
>
> This is explicitly talking about SBSA - is there any restriction with
> regard to non-SBSA systems? I can't think of any right now and this
> seems purely informational but it might be worth mentioning that
> non-SBSA systems might potentially have other states if the intention is
> to allow that.
It is informational, for nomenclature reasons. I do not think we have a
problem here, I will check if rewording is necessary.
> > +- state node
> > +
> > + Description: must be child of either the cpu-idle-states node or
> > + a state node.
> > +
> > + The state node name shall be "stateN", where N = {0, 1, ...} is
> > + the node number; state nodes which are siblings within a single common
> > + parent node must be given a unique and sequential N value, starting
> > + from 0.
>
> This came up with the CPU bindings as well but I'm really not convinced
> that making the naming of the nodes important is great - it's not normal
> for DT and it makes the usual enumeration code not work. Can we not
> just have a property for state number in the node instead and allow the
> name to be anything? It seems we even have the index property
> already...
Name can be anything now, acked.
> > + - compatible
> > + Usage: Required
> > + Value type: <stringlist>
> > + Definition: Must be "arm,cpu-idle-state".
>
> Do we really need this given that the location in the tree is fixed -
> what would we extend it with in future?
I do not think it hurts either, honestly. Unless there is a strong
reason to remove it I would leave it there.
> > + - index
> > + Usage: Required
> > + Value type: <u32>
> > + Definition: It represents an idle state index, starting from 2.
> > + Index 0 represents the processor state "running"
> > + and index 1 represents processor mode
> > + "idle_standby", entered by executing a wfi
> > + instruction (SBSA,[3]); indexes 0 and 1 are
> > + standard ARM states that need not be described.
>
> ...but other numbers are valid.
Now it is sequential {0,1,....} and I defined what it means in terms of
power consumption (ordering).
> > + - entry-method
> > + Usage: Required
> > + Value type: <stringlist>
> > + Definition: Describes the method by which a CPU enters the
> > + idle state. This property is required and must be
> > + one of:
> > +
> > + - "arm,psci-cpu-suspend"
> > + ARM PSCI firmware interface, CPU suspend
> > + method[2].
> > +
> > + - "[vendor],[method]"
> > + An implementation dependent string with
> > + format "vendor,method", where vendor is a string
> > + denoting the name of the manufacturer and
> > + method is a string specifying the mechanism
> > + used to enter the idle state.
>
> Might be worth reversing these - arm,psci-cpu-suspend is just an example
> of a (hopefully very widely used) vendor method. Given that everyone is
> supposed to be using PSCI might it even be worth making it the default
> and the property optional?
I think it is ok as it is, honestly. It is certainly not through this
property that psci will be mandated, it is just a way to describe an
entry method and it seems fine to me.
> > + - power-state
> > + Usage: See definition.
> > + Value type: <u32>
> > + Definition: Depends on the entry-method property value.
> > + If entry-method is "arm,psci-cpu-suspend":
> > + # Property is required and represents
> > + psci-power-state parameter. Please refer to
> > + [2] for PSCI bindings definition.
>
> Should we just have the entry method nodes define their own properties
> for this stuff? I guess this goes back to the comment I made on some
> other binding that it might be cleaner to have an explicit PSCI binding
> rather than put PSCI explicitly in indiidual bindings.
I added to the PSCI bindings in this series. OK, I can add something like:
"Refer to entry-method bindings for the property value definition".
> > + - entry-latency
> > + Usage: Required
> > + Value type: <prop-encoded-array>
> > + Definition: u32 value representing worst case latency
> > + in microseconds required to enter the idle state.
>
> Why is this defined as an array?
For possible extensions.
> > + - cache-state-retained
> > + Usage: See definition
> > + Value type: <none>
> > + Definition: if present cache memory is retained on power down,
> > + otherwise it is lost.
>
> Might be better to define which caches?
>
> > + STATE1: state1 {
> > + compatible = "arm,cpu-idle-state";
>
> Even if we stick with the node name being meaningful it'd be nice to
> replace the STATEN here with a meaningful state name to improve
> legibility.
Ok I will add this to v4.
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH 2/5] ARM: shmobile: r8a7791: add i2c master nodes to dtsi
From: Wolfram Sang @ 2014-02-17 10:25 UTC (permalink / raw)
To: Magnus Damm
Cc: Geert Uytterhoeven, SH-Linux,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
Simon Horman
In-Reply-To: <CANqRtoRumYg15MnZJyow67h=SGSt079GAuY+e5S9KwK-Z0wc9A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 317 bytes --]
> >> realize that there is no shortcut for adding the compatible string to
> >> the driver. It has to be done sooner or later anyway.
> >
> > OK, can be seen this way. As said before, I'll resend.
>
> Thanks! I'd be happy to discuss this more over a beverage in the not
> so distant future! =)
Yay! :)
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] ARM: shmobile: r8a7791: add i2c master nodes to dtsi
From: Magnus Damm @ 2014-02-17 10:11 UTC (permalink / raw)
To: Wolfram Sang
Cc: Geert Uytterhoeven, SH-Linux,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
Simon Horman
In-Reply-To: <20140217100807.GA4424@katana>
On Mon, Feb 17, 2014 at 7:08 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> It won't break because anyone who modifies r8a7790 support need to
>> make it specific to that SoC to not pull in r8a7791 into some
>> assumption.
>
> Geert's assumption was that exactly this can be forgotten. While this is
> true, my point was that this is a bug. And as it turned out, neither
> this or that solution helps the case :) It needs to be fixed or done
> properly right from the beginning.
>
>> realize that there is no shortcut for adding the compatible string to
>> the driver. It has to be done sooner or later anyway.
>
> OK, can be seen this way. As said before, I'll resend.
Thanks! I'd be happy to discuss this more over a beverage in the not
so distant future! =)
/ magnus
^ permalink raw reply
* Re: [PATCH RFC v3 3/3] Documentation: arm: define DT idle states bindings
From: Lorenzo Pieralisi @ 2014-02-17 10:11 UTC (permalink / raw)
To: Sebastian Capella
Cc: Mark Rutland, Mike Turquette, Tomasz Figa, Mark Hambleton,
Russell King, Nicolas Pitre, Daniel Lezcano,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
Dave P Martin, Charles Garcia-Tobin, devicetree@vger.kernel.org,
Kevin Hilman, linux-pm@vger.kernel.org, Kumar Gala, Rob Herring,
Vincent Guittot, Antti Miettinen, pveerapa@broadcom.com,
Peter De Schrijver
In-Reply-To: <20140215012215.11460.5931@capellas-linux>
On Sat, Feb 15, 2014 at 01:22:15AM +0000, Sebastian Capella wrote:
> Quoting Lorenzo Pieralisi (2014-02-14 03:27:56)
> > On Fri, Feb 14, 2014 at 12:29:26AM +0000, Sebastian Capella wrote:
> > > Quoting Lorenzo Pieralisi (2014-02-13 04:47:32)
> > >
> > > > Such as ? "idle-states" ?
> > >
> > > That sounds good to me. Our preference is for idle-states to be used
> > > for name of the idle-states.txt node.
> >
> > Ok, so s/cpu-idle-state/idle-state everywhere, inclusive of state nodes
> > compatible properties ("arm,cpu-idle-state" becomes "arm,idle-state") ?
>
> No, we were looking to make sure that the cpu-idle-states defined in
> cpus.txt:
>
> - cpu node
> - cpu-idle-states
>
> And the cpu-idle-states defined in idle-states.txt:
>
> - cpu-idle-states node
>
> Did not use the same name, and instead had different, unique names.
>
> Our preference was to change only the idle-states.txt name.
Ok, done.
> > I do not like that, but I can remove the naming scheme and let people
> > find a naming scheme that complies with DT rules (nodes within a parent
> > must have a unique name). Not sure this would make dts more readable, but
> > I do not think it is a problem either.
>
> In the current implementation for cpuidle, we have a descriptive c-state
> name. As long as we can get this kind of functionality using the node
> name this seems fine to me.
Ok, now the naming scheme follows standard device tree naming, so it is
up to platforms to find descriptive names.
[...]
> > What I will do: move the entry-method to top-level cpu-idle-states node
> > (soon to be idle-states node) and add a property there:
> >
> > "arm,has-idlewfi"
> >
> > which allows me to prevent people from adding an explicit state that just
> > executes the wfi instruction on entry.
> >
> > This way we can have a *global* entry-method, and we can also detect if the
> > platform supports wfi in its bare form.
> >
> > Yes, index can start from 0, disallowing 0 and 1 was a (odd) way to prevent
> > people from adding wfi to DT. If the platform supports simple idlestandby
> > (ie wfi) it has to add the boolean property above.
> >
> > How does that sound ?
>
> In general I'm ok with indexing as you have it, even if only to specify
> an ordering of states by power. I even thought maybe you could call it
> order or sort-order or something, to help people understand you won't
> use it as an array index or name, but I don't think it's a big deal
> either way.
>
> Do platforms remove support for WFI? If they do, is this detectible
> somehow directly from the ARM without relying on DTS? It seems like
> a comment is enough to discourage people from defining a wfi state.
> Then eventually implement a common code path for idle. I'm fine not
> specifying this flag, but if you feel it can be useful I don't object.
Yeah, wfi flag won't be there. If we ever need it we will add it later.
index seems fine to me, it is well defined and as long as we know what
it means it is all sorted, unless someone has strong opinion against it.
Thanks !!
Lorenzo
^ permalink raw reply
* Re: [PATCH 2/5] ARM: shmobile: r8a7791: add i2c master nodes to dtsi
From: Wolfram Sang @ 2014-02-17 10:08 UTC (permalink / raw)
To: Magnus Damm
Cc: Geert Uytterhoeven, SH-Linux,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
Simon Horman
In-Reply-To: <CANqRtoRgMaAzzvD9OK+=_J2Or5HZHzg4RUb899-bXjM7LKHSyw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 581 bytes --]
> It won't break because anyone who modifies r8a7790 support need to
> make it specific to that SoC to not pull in r8a7791 into some
> assumption.
Geert's assumption was that exactly this can be forgotten. While this is
true, my point was that this is a bug. And as it turned out, neither
this or that solution helps the case :) It needs to be fixed or done
properly right from the beginning.
> realize that there is no shortcut for adding the compatible string to
> the driver. It has to be done sooner or later anyway.
OK, can be seen this way. As said before, I'll resend.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v3 7/7] ARM: dts: imx6sl: Add power-domain information to gpc node
From: Philipp Zabel @ 2014-02-17 10:05 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Mark Rutland, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel
In-Reply-To: <1392631503-17283-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
The PGC that is part of GPC controls isolation and power sequencing of the
power domains. The PU power domain will be handled by the generic pm domain
framework and needs a phandle to the PU regulator to turn off power when
the domain is disabled.
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
arch/arm/boot/dts/imx6sl.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 28558f1..774e1fb 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -529,9 +529,27 @@
};
gpc: gpc@020dc000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
compatible = "fsl,imx6sl-gpc", "fsl,imx6q-gpc";
reg = <0x020dc000 0x4000>;
interrupts = <0 89 0x04>;
+ pu-supply = <®_pu>;
+
+ pd_display: display-power-domain@020dc240 {
+ compatible = "fsl,imx6q-power-domain";
+ reg = <0x020dc240 0x10>;
+ };
+
+ pd_pu: pu-power-domain@020dc260 {
+ compatible = "fsl,imx6q-power-domain";
+ reg = <0x020dc260 0x10>;
+ };
+
+ pd_arm: cpu-power-domain@020dc2a0 {
+ compatible = "fsl,imx6q-power-domain";
+ reg = <0x020dc2a0 0x10>;
+ };
};
gpr: iomuxc-gpr@020e0000 {
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 6/7] ARM: dts: imx6qdl: Add power-domain information to gpc node
From: Philipp Zabel @ 2014-02-17 10:05 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Mark Rutland, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel
In-Reply-To: <1392631503-17283-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
The PGC that is part of GPC controls isolation and power sequencing of the
power domains. The PU power domain will be handled by the generic pm domain
framework and needs a phandle to the PU regulator to turn off power when
the domain is disabled.
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
arch/arm/boot/dts/imx6qdl.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 253d82c..fd1be55 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -598,9 +598,22 @@
};
gpc: gpc@020dc000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
compatible = "fsl,imx6q-gpc";
reg = <0x020dc000 0x4000>;
interrupts = <0 89 0x04 0 90 0x04>;
+ pu-supply = <®_pu>;
+
+ pd_pu: pu-power-domain@020dc260 {
+ compatible = "fsl,imx6q-power-domain";
+ reg = <0x020dc260 0x10>;
+ };
+
+ pd_arm: cpu-power-domain@020dc2a0 {
+ compatible = "fsl,imx6q-power-domain";
+ reg = <0x020dc2a0 0x10>;
+ };
};
gpr: iomuxc-gpr@020e0000 {
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 5/7] ARM: dts: imx6qdl: Allow disabling the PU regulator, add a enable ramp delay
From: Philipp Zabel @ 2014-02-17 10:05 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Mark Rutland, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel
In-Reply-To: <1392631503-17283-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
The PU regulator is enabled during boot, but not necessarily always-on.
It can be disabled by the generic pm domain framework when the PU power
domain is shut down. The ramp delay of 150 us might be a bit conservative,
the value is taken from the Freescale kernel.
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
arch/arm/boot/dts/imx6qdl.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index fb28b2e..253d82c 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -515,7 +515,8 @@
regulator-name = "vddpu";
regulator-min-microvolt = <725000>;
regulator-max-microvolt = <1450000>;
- regulator-always-on;
+ regulator-enable-ramp-delay = <150>;
+ regulator-boot-on;
anatop-reg-offset = <0x140>;
anatop-vol-bit-shift = <9>;
anatop-vol-bit-width = <5>;
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 4/7] ARM: imx6: gpc: Add observed worst case latencies
From: Philipp Zabel @ 2014-02-17 10:05 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Mark Rutland, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel
In-Reply-To: <1392631503-17283-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
This avoids the "... latency exceeded, new value ..." warnings
emitted by the power domain framework code whenever the PU domain
is enabled or disabled.
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
arch/arm/mach-imx/gpc.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index e16e36a..9ca81d8 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -225,6 +225,8 @@ static struct generic_pm_domain imx6q_pu_domain = {
.name = "PU",
.power_off = imx6q_pm_pu_power_off,
.power_on = imx6q_pm_pu_power_on,
+ .power_off_latency_ns = 25000,
+ .power_on_latency_ns = 2000000,
};
int imx6q_pm_clk_add(struct device *dev)
@@ -289,6 +291,13 @@ int imx6q_pm_clk_remove(struct device *dev)
return 0;
}
+static struct gpd_timing_data pu_timing_data = {
+ .stop_latency_ns = 2000,
+ .start_latency_ns = 2000,
+ .save_state_latency_ns = 5000,
+ .restore_state_latency_ns = 20000000, /* VPU firmware reload */
+};
+
static int imx6q_pm_notifier_call(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -303,7 +312,7 @@ static int imx6q_pm_notifier_call(struct notifier_block *nb,
if (!np || np != imx6q_pu_domain.of_node)
return NOTIFY_DONE;
- ret = pm_genpd_of_add_device(np, dev);
+ ret = __pm_genpd_of_add_device(np, dev, &pu_timing_data);
if (ret)
dev_err(dev, "failed to add to power domain: %d\n",
ret);
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 3/7] ARM: imx6: gpc: Add pm clock support to PU power domain
From: Philipp Zabel @ 2014-02-17 10:04 UTC (permalink / raw)
To: Shawn Guo
Cc: Rob Herring, Mark Rutland, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Philipp Zabel
In-Reply-To: <1392631503-17283-1-git-send-email-p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Drivers still handle clocks themselves, we only enable pm clocks of the
GPU and VPU devices in the PU power domain temporarily during powerup
so that the reset machinery can work.
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
arch/arm/mach-imx/gpc.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index c61126c..e16e36a 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -18,6 +18,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
#include <linux/pm_domain.h>
#include <linux/regulator/consumer.h>
#include <linux/irqchip/arm-gic.h>
@@ -182,6 +183,7 @@ static int imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
static int imx6q_pm_pu_power_on(struct generic_pm_domain *genpd)
{
+ struct pm_domain_data *pdd;
int ret;
u32 val;
int sw, sw2iso;
@@ -192,6 +194,10 @@ static int imx6q_pm_pu_power_on(struct generic_pm_domain *genpd)
return ret;
}
+ /* Enable PM clocks for all devices in the PU domain */
+ list_for_each_entry(pdd, &genpd->dev_list, list_node)
+ pm_clk_resume(pdd->dev);
+
/* Gate off PU domain when GPU/VPU when powered down */
writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
@@ -208,6 +214,10 @@ static int imx6q_pm_pu_power_on(struct generic_pm_domain *genpd)
/* Wait ISO + ISO2SW IPG clock cycles */
ndelay((sw + sw2iso) * 1000 / 66);
+ /* Disable PM clocks for all devices in the PU domain */
+ list_for_each_entry(pdd, &genpd->dev_list, list_node)
+ pm_clk_suspend(pdd->dev);
+
return 0;
}
@@ -217,6 +227,68 @@ static struct generic_pm_domain imx6q_pu_domain = {
.power_on = imx6q_pm_pu_power_on,
};
+int imx6q_pm_clk_add(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ const char *con_id;
+ struct clk *clk;
+ int i = 0;
+
+ /* Add and prepare named clocks */
+ while (!of_property_read_string_index(np, "clock-names", i, &con_id)) {
+ pm_clk_add(dev, con_id);
+ clk = of_clk_get(np, i);
+ if (!IS_ERR(clk)) {
+ clk_prepare(clk);
+ clk_put(clk);
+ }
+ i++;
+ }
+
+ /* If no named clocks are given, add and prepare unnamed clock */
+ if (i == 1 && of_find_property(dev->of_node, "clocks", NULL)) {
+ pm_clk_add(dev, NULL);
+ clk = of_clk_get(np, 0);
+ if (!IS_ERR(clk)) {
+ clk_prepare(clk);
+ clk_put(clk);
+ }
+ }
+
+ return 0;
+}
+
+int imx6q_pm_clk_remove(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ const char *con_id;
+ struct clk *clk;
+ int i = 0;
+
+ /* Remove and unprepare named clocks */
+ while (!of_property_read_string_index(np, "clock-names", i, &con_id)) {
+ pm_clk_remove(dev, con_id);
+ clk = of_clk_get(np, i);
+ if (!IS_ERR(clk)) {
+ clk_unprepare(clk);
+ clk_put(clk);
+ }
+ i++;
+ }
+
+ /* If no named clocks are given, remove and unprepare unnamed clock */
+ if (i == 1 && of_find_property(dev->of_node, "clocks", NULL)) {
+ pm_clk_remove(dev, NULL);
+ clk = of_clk_get(np, 0);
+ if (!IS_ERR(clk)) {
+ clk_unprepare(clk);
+ clk_put(clk);
+ }
+ }
+
+ return 0;
+}
+
static int imx6q_pm_notifier_call(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -235,6 +307,7 @@ static int imx6q_pm_notifier_call(struct notifier_block *nb,
if (ret)
dev_err(dev, "failed to add to power domain: %d\n",
ret);
+ imx6q_pm_clk_add(dev);
break;
case BUS_NOTIFY_UNBOUND_DRIVER:
genpd = dev_to_genpd(dev);
@@ -245,6 +318,7 @@ static int imx6q_pm_notifier_call(struct notifier_block *nb,
if (ret)
dev_err(dev, "failed to remove from power domain: %d\n",
ret);
+ imx6q_pm_clk_remove(dev);
break;
}
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
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