* Re: [PATCH v3 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Joel Stanley @ 2017-05-02 7:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
Jeremy Kerr, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree, Benjamin Herrenschmidt,
OpenBMC Maillist
In-Reply-To: <CAHp75Vd=oONhzF34Pe8WwYKBWWxcsaQtgJy66aFD68iNKTxEfg@mail.gmail.com>
On Tue, Apr 11, 2017 at 9:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Apr 10, 2017 at 7:04 AM, Joel Stanley <joel@jms.id.au> wrote:
>> From: Jeremy Kerr <jk@ozlabs.org>
>>
>> This change adds a driver for the 16550-based Aspeed virtual UART
>> device. We use a similar process to the of_serial driver for device
>> probe, but expose some VUART-specific functions through sysfs too.
>>
>> The VUART is two UART 'front ends' connected by their FIFO (no actual
>> serial line in between). One is on the BMC side (management controller)
>> and one is on the host CPU side.
>>
>> This driver is for the BMC side. The sysfs files allow the BMC
>> userspace, which owns the system configuration policy, to specify at
>> what IO port and interrupt number the host side will appear to the host
>> on the Host <-> BMC LPC bus. It could be different on a different system
>> (though most of them use 3f8/4).
>>
>> OpenPOWER host firmware doesn't like it when the host-side of the
>> VUART's FIFO is not drained. This driver only disables host TX discard
>> mode when the port is in use. We set the VUART enabled bit when we bind
>> to the device, and clear it on unbind.
>>
>> We don't want to do this on open/release, as the host may be using this
>> bit to configure serial output modes, which is independent of whether
>> the devices has been opened by BMC userspace.
>
>> +static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled)
>> +{
>> + u8 reg;
>> +
>> + reg = readb(vuart->regs + ASPEED_VUART_GCRA);
>
>> + reg &= ~ASPEED_VUART_GCRA_VUART_EN;
>> + if (enabled)
>> + reg |= ASPEED_VUART_GCRA_VUART_EN;
>
> Usually the pattern is
> if (something)
> set x bit;
> else
> clear x bit;
>
> It would make it one operation in any case and a bit more understandable.
I have made these ordering changes you requested.
>
>> + port.port.irq = irq_of_parse_and_map(np, 0);
>
> The benefit of use platform_get_irq() is to get rid of some OF specific headers.
It's an of driver, and this function does exactly what we require. I
have left it in for v4.
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH v3 1/2] ASoC: stm32: add bindings for SAI
From: Olivier MOYSAN @ 2017-05-02 7:45 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, Alexandre TORGUE,
linux-kernel@vger.kernel.org, Arnaud POULIQUEN, tiwai@suse.com,
lgirdwood@gmail.com, broonie@kernel.org,
mcoquelin.stm32@gmail.com, Benjamin GAIGNARD,
linux-arm-kernel@lists.infradead.org, kernel@stlinux.com
In-Reply-To: <20170428205358.fxjqaki44xqim4ta@rob-hp-laptop>
Hello Rob,
On 04/28/2017 10:53 PM, Rob Herring wrote:
> On Mon, Apr 10, 2017 at 05:19:55PM +0200, olivier moysan wrote:
>> This patch adds documentation of device tree bindings for the
>> STM32 SAI ASoC driver.
>>
>> Signed-off-by: olivier moysan <olivier.moysan@st.com>
>> ---
>> .../devicetree/bindings/sound/st,stm32-sai.txt | 89 ++++++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/st,stm32-sai.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-sai.txt b/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
>> new file mode 100644
>> index 0000000..c59a3d7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/st,stm32-sai.txt
>> @@ -0,0 +1,89 @@
>> +STMicroelectronics STM32 Serial Audio Interface (SAI).
>
> [...]
>
>> + sai1b: audio-controller@40015824 {
>> + #sound-dai-cells = <0>;
>> + compatible = "st,stm32-sai-sub-b";
>> + reg = <0x40015824 0x1C>;
>> + clocks = <&rcc 1 CLK_SAI2>;
>> + clock-names = "sai_ck";
>> + dmas = <&dma2 5 0 0x400 0x0>;
>> + dma-names = "tx";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_sai1b>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + sai1b_port: port@0 {
>> + reg = <0>;
>> + cpu_endpoint: endpoint {
>> + remote-endpoint = <&codec_endpoint>;
>> + audio-graph-card,format = "i2s";
>> + audio-graph-card,bitclock-master = <&codec_endpoint>;
>> + audio-graph-card,frame-master = <&codec_endpoint>;
>
> These property names are wrong.
>
I have taken into account this comment (and previous ones).
They will be included in next update of this patch set.
>> + };
>> + };
>> + };
>> + };
>> +};
>> +
>> +audio-codec {
>> + codec_port: port {
>> + codec_endpoint: endpoint {
>> + remote-endpoint = <&cpu_endpoint>;
>> + };
>> + };
>> +};
>> --
>> 1.9.1
>>
Best regards
Olivier
^ permalink raw reply
* BENEFIT
From: Mrs Julie Leach @ 2017-05-02 7:37 UTC (permalink / raw)
To: Recipients
You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact(julieleach93-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) for claims.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 7/9] power: Adds support for Smart Battery System Manager
From: Sebastian Reichel @ 2017-05-02 7:12 UTC (permalink / raw)
To: Phil Reid
Cc: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, peda-koto5C5qi+TLoDKTGw+V6w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Karl-Heinz Schneider
In-Reply-To: <bf4cf813-609c-1d92-2329-35e9176d0d28-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 472 bytes --]
Hi Phil,
On Tue, May 02, 2017 at 10:20:22AM +0800, Phil Reid wrote:
> In regards the le to cpu conversion. grepping the supply folder
> finds a couple of other suspicious cases.
>
> sbs-battery & the bq24735-cahrger do similar things.
>
> thoughts on if they need to be corrected?
Yes, those drivers look incorrect. I guess nobody noticed, since
most people use little endian machines nowadays (so le16_to_cpu and
cpu_to_le16 do nothing).
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: FW: [PATCH 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe
From: Ryder Lee @ 2017-05-02 7:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rob Herring, linux-pci-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Bjorn Helgaas,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4BAFE512E7223A4B91F29C40AC7A579616B5155E-FTLr7L0+/BWGRgb7Xm6L22/AB3i2Q03W@public.gmane.org>
Hi Arnd,
> 2017-04-28 19:41 GMT+08:00 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>
> On Fri, Apr 28, 2017 at 4:46 AM, Ryder Lee
> <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > On Thu, 2017-04-27 at 21:06 +0200, Arnd Bergmann wrote:
> >> On Wed, Apr 26, 2017 at 10:10 AM, Ryder Lee
> <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >> > On Tue, 2017-04-25 at 14:18 +0200, Arnd Bergmann wrote:
> >> >> On Sun, Apr 23, 2017 at 10:19 AM, Ryder Lee
> <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >> Are any of the registers the same at all, e.g. for MSI
> handling?
> >
> > No, It doesn't support MSI. All I can do is using the
> registers that designer provide to me. The others are inviable
> for software. So I treat it as different hardware.
> Furthermore, we hope that we can put all mediatek drivers
> together regardless of in-house IP or lincense IP
> >
> > We have no particular IP name but just use chip name to call
> it. So I will temporarily use "mediatek,gen2v1-pcie" in patch
> v1.
>
> I think using the chip name as in the first version of your
> patch name is better then, in particular since the 'gen2v1'
> would not be an actual version number but just say which
> variant got merged into mainline first.
Okay, i will correct it.
> A related question would be on how general we want the binding
> to be.
> Your binding text starts out by describing that there are
> three root ports and what their capabilities are.
>
> If you think there might be other (existing or future) chips
> that use the same binding and driver, then being a little more
> abstract could help in the long run.
Thanks for reminding me. If we decide to use the same driver in the
future, we will have a internal discussion about it.
Ryder.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: phy: Add documentation for Mediatek PCIe PHY
From: Ryder Lee @ 2017-05-02 7:08 UTC (permalink / raw)
To: Rob Herring
Cc: Kishon Vijay Abraham I, Matthias Brugger, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20170428175200.laakck7ill2lxtxc@rob-hp-laptop>
On Fri, 2017-04-28 at 12:52 -0500, Rob Herring wrote:
> On Sun, Apr 23, 2017 at 04:17:33PM +0800, Ryder Lee wrote:
> > Add documentation for PCIe PHY available in MT7623 series SoCs.
> >
> > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> > ---
> > .../devicetree/bindings/phy/phy-mt7623-pcie.txt | 67 ++++++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt b/Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt
> > new file mode 100644
> > index 0000000..27a9253
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-mt7623-pcie.txt
> > @@ -0,0 +1,67 @@
> > +Mediatek MT7623 PCIe PHY
> > +-----------------------
> > +
> > +Required properties:
> > + - compatible: Should contain "mediatek,mt7623-pcie-phy"
> > + - #phy-cells: must be 0
> > + - clocks: Must contain an entry in clock-names.
> > + See ../clocks/clock-bindings.txt for details.
> > + - clock-names: Must be "refclk"
> > + - resets: Must contain an entry in reset-names.
> > + See ../reset/reset.txt for details.
> > + - reset-names: Must be "phy"
> > +
> > +Optional properties:
> > + - phy-switch: The PHY on PCIe port2 is shared with USB u3phy2. If you
> > + want to enable port2, you should contain it.
>
> Need to state what the value is (i.e. a phandle to ?). Also needs a
> vendor prefix.
I will correct it.
> > +
> > +Example:
> > +
> > + pcie0_phy: pciephy@1a149000 {
>
> pcie-phy@...
Okay.
> > + compatible = "mediatek,mt7623-pcie-phy";
> > + reg = <0 0x1a149000 0 0x1000>;
> > + clocks = <&clk26m>;
> > + clock-names = "pciephya_ref";
> > + #phy-cells = <0>;
> > + status = "disabled";
>
> Don't show status in examples.
I will drop it all.
> > + };
> > +
> > + pcie1_phy: pciephy@1a14a000 {
> > + compatible = "mediatek,mt7623-pcie-phy";
> > + reg = <0 0x1a14a000 0 0x1000>;
> > + clocks = <&clk26m>;
> > + clock-names = "pciephya_ref";
> > + #phy-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > + pcie2_phy: pciephy@1a244000 {
> > + compatible = "mediatek,mt7623-pcie-phy";
> > + reg = <0 0x1a244000 0 0x1000>;
> > + clocks = <&clk26m>;
> > + clock-names = "pciephya_ref";
> > + #phy-cells = <0>;
> > +
> > + phy-switch = <&hifsys>;
> > + status = "disabled";
> > + };
> > +
> > +Specifying phy control of devices
> > +---------------------------------
> > +
> > +Device nodes should specify the configuration required in their "phys"
> > +property, containing a phandle to the phy node and phy-names.
> > +
> > +Example:
> > +
> > +#include <dt-bindings/phy/phy.h>
> > +
> > +pcie: pcie@1a140000 {
> > + ...
> > + pcie@1,0 {
> > + ...
> > + phys = <&pcie0_phy>;
> > + phy-names = "pcie-phy0";
> > + }
> > + ...
> > +};
Thanks for your reviews.
> > --
> > 1.9.1
> >
^ permalink raw reply
* Re: [PATCH v1 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe
From: Ryder Lee @ 2017-05-02 7:07 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Helgaas, Arnd Bergmann, linux-pci-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Red Hung
In-Reply-To: <20170428210924.y6xdv4trqbmyn5me@rob-hp-laptop>
On Fri, 2017-04-28 at 16:09 -0500, Rob Herring wrote:
> On Fri, Apr 28, 2017 at 05:10:34PM +0800, Ryder Lee wrote:
> > Add binding document for Mediatek PCIe Gen2 v1 host controller driver.
> >
> > Signed-off-by: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> > .../bindings/pci/mediatek,gen2v1-pcie.txt | 174 +++++++++++++++++++++
> > 1 file changed, 174 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt
> > new file mode 100644
> > index 0000000..545d8cf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/mediatek,gen2v1-pcie.txt
> > @@ -0,0 +1,174 @@
> > +Mediatek Gen2 V1 PCIe controller
> > +
> > +PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
> > +ports supports a Gen2 1-lane Link. It includes one Host/PCI bridge and 3
> > +PCIe MAC. Each port has PIPE interface to PHY. There are 3 bus master for
> > +data access and 1 bus slave for Configuration and Status Register access.
> > +
> > +This controller is available on MT7623 series SoCs.
> > +
> > +Required properties:
> > +- compatible: Should contain "mediatek,gen2v1-pcie".
> > +- device_type: Must be "pci"
> > +- reg: Base addresses and lengths of the PCIe controller.
> > +- #address-cells: Address representation for root ports (must be 3)
> > + - cell 0 specifies the bus and device numbers of the root port:
> > + [23:16]: bus number
> > + [15:11]: device number
> > + - cell 1 denotes the upper 32 address bits and should be 0
> > + - cell 2 contains the lower 32 address bits and is used to translate to the
> > + CPU address space
>
> This is all standard PCI bus binding. You don't need to define it here.
> "must be 3" is sufficient.
Okay.
> > +- #size-cells: Size representation for root ports (must be 2)
> > +- #interrupt-cells: Size representation for interrupts (must be 1)
> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> > + entry for each entry in the interrupt-names property.
>
> Where's interrupt-names?
I will add it.
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > + Please refer to the standard PCI bus binding document for a more detailed
> > + explanation.
> > +- clocks: Must contain an entry for each entry in clock-names.
> > + See ../clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > + - free_ck :for reference clock of PCIe subsys
> > + - sys_ck0 :for clock of Port0 MAC
> > + - sys_ck1 :for clock of Port1 MAC
> > + - sys_ck2 :for clock of Port2 MAC
> > +- resets: Must contain an entry for each entry in reset-names.
> > + See ../reset/reset.txt for details.
> > +- reset-names: Must include the following entries:
> > + - pcie-rst0 :port0 reset
> > + - pcie-rst1 :port1 reset
> > + - pcie-rst2 :port2 reset
> > +- phys: list of PHY specifiers (used by generic PHY framework)
> > +- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
> > + number of PHYs as specified in *phys* property.
> > +- power-domains: A phandle and power domain specifier pair to the power domain
> > + which is responsible for collapsing and restoring power to the peripheral
> > +- bus-range: Range of bus numbers associated with this controller
> > +- ranges: Describes the translation of addresses for root ports and standard
> > + PCI regions. The entries must be 6 cells each, where the first three cells
> > + correspond to the address as described for the #address-cells property
> > + above, the fourth cell is the physical CPU address to translate to and the
> > + fifth and six cells are as described for the #size-cells property above.
>
> Don't need to define what ranges is here, just what the entries should
> be:
Okay.
> > + - The first three entries are expected to translate the addresses for the root
> > + port registers, which are referenced by the assigned-addresses property of
> > + the root port nodes (see below).
> > + - The remaining entries setup the mapping for the standard I/O and memory
> > + regions.
> > + Please refer to the standard PCI bus binding document for a more detailed
> > + explanation.
> > +
> > +In addition, the device tree node must have sub-nodes describing each
> > +PCIe port interface, having the following mandatory properties:
> > +
> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus number
> > + and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- #interrupt-cells: Size representation for interrupts (must be 1)
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > + Please refer to the standard PCI bus binding document for a more detailed
> > + explanation.
> > +- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> > + property is sufficient.
> > +- num-lanes: Number of lanes to use for this port.
> > +
> > +Examples:
> > +
> > +SoC dtsi:
>
> Don't show the board vs. SoC split in examples. And drop all the status
> properties.
Okay i will drop it all.
> > +
> > + hifsys: syscon@1a000000 {
> > + compatible = "mediatek,mt7623-hifsys", "syscon";
> > + reg = <0 0x1a000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + #reset-cells = <1>;
> > + };
> > +
> > + pcie: pcie-controller@1a140000 {
> > + compatible = "mediatek,gen2v1-pcie";
> > + device_type = "pci";
> > + reg = <0 0x1a140000 0 0x1000>; /* PCIe shared registers */
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + #interrupt-cells = <1>;
> > + interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>,
> > + <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>,
> > + <GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;
> > + interrupt-map-mask = <0xf800 0 0 0>;
> > + interrupt-map = <0x0000 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>,
> > + <0x0800 0 0 0 &gic GIC_SPI 194 IRQ_TYPE_NONE>,
> > + <0x1000 0 0 0 &gic GIC_SPI 195 IRQ_TYPE_NONE>;
> > + clocks = <&topckgen CLK_TOP_ETHIF_SEL>,
> > + <&hifsys CLK_HIFSYS_PCIE0>,
> > + <&hifsys CLK_HIFSYS_PCIE1>,
> > + <&hifsys CLK_HIFSYS_PCIE2>;
> > + clock-names = "free_ck", "sys_ck0", "sys_ck1", "sys_ck2";
> > + resets = <&hifsys MT2701_HIFSYS_PCIE0_RST>,
> > + <&hifsys MT2701_HIFSYS_PCIE1_RST>,
> > + <&hifsys MT2701_HIFSYS_PCIE2_RST>;
> > + reset-names = "pcie-rst0", "pcie-rst1", "pcie-rst2";
> > + phys = <&pcie0_phy>, <&pcie1_phy>, <&pcie2_phy>;
> > + phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
> > + power-domains = <&scpsys MT2701_POWER_DOMAIN_HIF>;
> > + bus-range = <0x00 0xff>;
> > + ranges = <0x82000000 0 0x1a142000 0 0x1a142000 0 0x1000 /* Port0 registers */
> > + 0x82000000 0 0x1a143000 0 0x1a143000 0 0x1000 /* Port1 registers */
> > + 0x82000000 0 0x1a144000 0 0x1a144000 0 0x1000 /* Port2 registers */
> > + 0x81000000 0 0x1a160000 0 0x1a160000 0 0x00010000 /* I/O space */
> > + 0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>; /* memory space */
> > + status = "disabled";
> > +
> > + pcie@1,0 {
> > + device_type = "pci";
> > + assigned-addresses = <0x82000000 0 0x1a142000 0 0x1000>;
> > + reg = <0x0000 0 0 0 0>;
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + #interrupt-cells = <1>;
> > + interrupt-map-mask = <0 0 0 0>;
> > + interrupt-map = <0 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>;
> > + ranges;
> > + num-lanes = <1>;
> > + status = "disabled";
> > + };
> > +
> > + pcie@2,0 {
> > + device_type = "pci";
> > + assigned-addresses = <0x82000800 0 0x1a143000 0 0x1000>;
> > + reg = <0x0800 0 0 0 0>;
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + #interrupt-cells = <1>;
> > + interrupt-map-mask = <0 0 0 0>;
> > + interrupt-map = <0 0 0 0 &gic GIC_SPI 194 IRQ_TYPE_NONE>;
> > + ranges;
> > + num-lanes = <1>;
> > + status = "disabled";
> > + };
> > +
> > + pcie@3,0 {
> > + device_type = "pci";
> > + assigned-addresses = <0x82001000 0 0x1a144000 0 0x1000>;
> > + reg = <0x1000 0 0 0 0>;
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + #interrupt-cells = <1>;
> > + interrupt-map-mask = <0 0 0 0>;
> > + interrupt-map = <0 0 0 0 &gic GIC_SPI 195 IRQ_TYPE_NONE>;
> > + ranges;
> > + num-lanes = <1>;
> > + status = "disabled";
> > + };
> > + };
> > +
> > +Board dts:
> > +
> > + &pcie {
> > + status = "okay";
> > +
> > + pcie@1,0 {
> > + status = "okay";
> > + };
> > + };
> > --
> > 1.9.1
Thanks for your review.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: rockchip-dw-mshc: add optional rockchip,default-num-phases
From: Shawn Lin @ 2017-05-02 7:03 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Ziyuan Xu,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Doug Anderson, Jaehoon Chung,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170428133403.mmdvtp3ohff6v7gh@rob-hp-laptop>
Hi Rob,
在 2017/4/28 21:34, Rob Herring 写道:
> On Wed, Apr 19, 2017 at 05:00:33PM +0800, Shawn Lin wrote:
>> By default, dw_mmc-rockchip will execute tuning for each degree.
>> So we won't miss every point of the good sample windows. However,
>> probably the phases are linear inside the good sample window.
>> Actually we don't need to do tuning for each degree so that we could
>> save some time, for instance, probe the driver or resume from S3.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> index 520d61d..ea47ec0 100644
>> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>> @@ -31,6 +31,10 @@ Optional Properties:
>> probing, low speeds or in case where all phases work at tuning time.
>> If not specified 0 deg will be used.
>>
>> +* rockchip,default-num-phases: The default number of times that the host
>> + execute tuning when needed. If not specified, the host will do tuning
>> + for 360 times, namely tuning for each degree.
>
> How is it default when you specify it? I would think default here is
> 360.
>
I don't get your point here. Do you mean the name, default-num-phases,
isn't correct, so I need another one?
> Should this be common?
>
> Rob
>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* Re: [PATCH 2/2] mmc: dw_mmc-rockchip: parse rockchip,default-num-phases from DT
From: Shawn Lin @ 2017-05-02 6:58 UTC (permalink / raw)
To: Doug Anderson
Cc: Jaehoon Chung, Ulf Hansson, Rob Herring,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ziyuan Xu,
open list:ARM/Rockchip SoC...,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAD=FV=Uyqvzg5H+Mg5RtQAWiCxVARx7uB4jxPe=2fcSmJQoOjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Doug,
在 2017/4/25 0:18, Doug Anderson 写道:
> Hi,
>
> On Wed, Apr 19, 2017 at 6:21 PM, Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> Hi Doug,
>>
>> 在 2017/4/20 4:19, Doug Anderson 写道:
>>>
>>> Hi,
>>>
>>> On Wed, Apr 19, 2017 at 2:00 AM, Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>> wrote:
>>>>
>>>> Currently we unconditionally do tuning for each degree, which
>>>> costs 900ms for each boot and resume.
>>>>
>>>> May someone argue that this is a question of accuracy VS time. But I
>>>> would say it's a trick of how we need to do decision for our boards.
>>>> If we don't care the time we spend at all, we could definitely do tuning
>>>> for each degree. But when we need to improve the user experience, for
>>>> instance, speed up resuming from S3, we should also have the right to
>>>> do that. This patch add parsing "rockchip,default-num-phases", for folks
>>>> to specify the number of doing tuning. If not specified, 360 will be used
>>>> as before.
>>>>
>>>> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>>>
>>>> ---
>>>>
>>>> drivers/mmc/host/dw_mmc-rockchip.c | 48
>>>> ++++++++++++++++++++++++--------------
>>>> 1 file changed, 30 insertions(+), 18 deletions(-)
>>>
>>>
>>> No huge objection here, but I do remember we ended up at the 360
>>> phases due to some of the craziness with dw_mmc delay elements on
>>> rk3288. IIRC one of the big problems was that the delay elements
>>> changed _a lot_ with the "LOGIC" voltage and we tweaked the voltage at
>>> runtime for DDR DVFS. That imposed an extra need to be very accurate
>>> on that SoC, at least on any board that was designed to support DDR
>>> DVFS.
>>>
>>
>> Not just with the vdd_logic but also with the process of Soc.
>> To better guaratee the accuracy, firstly we use delay element to do
>> tuning and then convert it to be combination of degree + delay element.
>> But as the dalay elements aren't accuracy themself, so all the math we
>> do here is trick.
>
> Yup. I brought up the vdd logic specifically because it's something
> that can make the phases change quite dramatically on the same machine
> between the time you tuned and the time you used it.
>
>
>>> I also remember there being some weirdness on the Rockchip
>>> implementation where there was a certain set of phases that the MMC
>>> controller was essentially "blind". This blind spot was in the middle
>>> of an otherwise good range of points. Unfortunately this blind spot
>>> was somewhat hard to detect properly because it was not very big.
>>> ...the variability of the delay elements meant that there could be big
>>> ranges where we weren't getting any good test coverage, but also the
>>> fact that they changed with the LOGIC voltage might mean that we
>>> weren't in the "blind" spot and then suddenly we were.
>>
>>
>> I undertand all of these as I was suffering from it when bringing up
>> RK3288.
>>
>>>
>>> One other note is that i remember that the vast majority of time spent
>>> tuning was dealing with "bad" phases, not dealing with "good" phases.
>>> If you're looking to speed things up, maybe finding a way to make
>>> "bad" phases fail faster would be wise? I think one of the reasons
>>> bad phases failed so slowly is because the dw_mmc timeouts are all so
>>> long.
>>
>>
>> Good point. I haven't thought of speeding up the handle of bad phases,
>> but I will take a look at this.
>>
>>>
>>> Oh, and I guess one last note is that I have no idea if folks will
>>> like the device bindings here. Part of me thinks it should be
>>> something more "symbolic" like "rockchip,need-accurate-tuning" or
>>> something like that. I guess I'd let the DT experts chime in.
>>>
>>>
>>> So I guess to summarize:
>>> * On rk3288 boards w/ DDR DVFS (or any other similar boards), 360
>>> seems to provide real benefit.
>>> * On other boards, probably you can get by with fewer phases.
>>>
>>
>> I would try to say it's a question of "900ms for a single time" VS.
>> "some of discrete tuning cost for more chance to do retune".
>>
>> (1)We could try to do a more accurate tuning process and spends 900ms.
>> Then we have less chance to do retune later.
>>
>> (2)We do a rough tuning and have more chance to do retune later.
>
> Ah, interesting point. I haven't used newer versions of Linux much,
> but I seem to remember that they will automatically retune sometimes
> if they see errors. That makes your strategy a bit more valid.
>
>
>> I also would say that this is a game , and we can't say which
>> one is better. Obvious now the "900ms" alwyas happens in the spot
>> routine, for instance, booting and resuming from S3.
>
> Is it really 900 ms? I don't quite remember it being that long, but I
> could be remembering incorrectly.
I saw the worst case was nearly 900ms. But mostly we need 600ms there.
>
> -Doug
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/2] regulator: Allow for asymmetric settling times
From: Laxman Dewangan @ 2017-05-02 6:53 UTC (permalink / raw)
To: Matthias Kaehlcke, Liam Girdwood, Mark Brown, Rob Herring,
Mark Rutland
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Anderson, Brian Norris
In-Reply-To: <20170501183715.35375-2-mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
On Tuesday 02 May 2017 12:07 AM, Matthias Kaehlcke wrote:
> Some regulators have different settling times for voltage increases and
> decreases. To avoid a time penalty on the faster transition allow for
> different settings for up- and downward transitions.
>
> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] regulator: DT: Add properties for asymmetric settling times
From: Laxman Dewangan @ 2017-05-02 6:51 UTC (permalink / raw)
To: Matthias Kaehlcke, Liam Girdwood, Mark Brown, Rob Herring,
Mark Rutland
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Douglas Anderson, Brian Norris
In-Reply-To: <20170501183715.35375-1-mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
On Tuesday 02 May 2017 12:07 AM, Matthias Kaehlcke wrote:
> Some regulators have different settling times for voltage increases and
> decreases. Add DT properties to define separate settling times for up-
> and downward voltage changes.
>
> Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC] Documentation/devicetree: Add specification for FSI busses
From: Jeremy Kerr @ 2017-05-02 6:25 UTC (permalink / raw)
To: Joel Stanley; +Cc: devicetree, OpenBMC Maillist
In-Reply-To: <CACPK8Xf2regdT4Y-TW4nOc1qYwDcH=zYPRqdjTqL9yvZH49bRA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Joel,
> Looks good to me Jeremy. One small question on the description, but
> please add Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> to future
> revisions.
Thanks for the review!
>> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
>> +engines within those slaves. However, we have a facility to match devicetree
>> +nodes to probed engines. This allows for fsi engines to expose non-probeable
>> +busses, which are then exposed by the device tree. For example, a FSI engine
>
> A nit: Can we can expose any device as part of the engine, not just a bus?
Absolutely. I'll clarify in a future version. Something like:
This allows for fsi engines to expose non-probeable busses, or for
drivers to access extra device properties, by data described in the
device tree.
Cheers,
Jeremy
--
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 v6 07/23] drivers/fsi: Implement slave initialisation
From: Joel Stanley @ 2017-05-02 6:24 UTC (permalink / raw)
To: Christopher Bostic
Cc: Rob Herring, Mark Rutland, Russell King,
rostedt-nx8X9YLhiw1AfugRpC6u6w, mingo-H+wXaHxf7aLQT0dZR+AlfA,
Greg KH, devicetree,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeremy Kerr,
Linux Kernel Mailing List, Andrew Jeffery, Alistair Popple,
Benjamin Herrenschmidt
In-Reply-To: <20170410194706.64280-8-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On Tue, Apr 11, 2017 at 5:16 AM, Christopher Bostic
<cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> Implement fsi_slave_init: if we can read a chip ID, create fsi_slave
> devices and register with the driver core.
>
> Includes changes from Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> ---
> drivers/fsi/fsi-core.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 6e1cfdf..c705ca2 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -17,9 +17,12 @@
> #include <linux/fsi.h>
> #include <linux/idr.h>
> #include <linux/module.h>
> +#include <linux/slab.h>
>
> #include "fsi-master.h"
>
> +#define FSI_SLAVE_SIZE_23b 0x800000
> +
> static DEFINE_IDA(master_ida);
>
> struct fsi_slave {
> @@ -114,11 +117,70 @@ static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
> addr, val, size);
> }
>
> +static void fsi_slave_release(struct device *dev)
> +{
> + struct fsi_slave *slave = to_fsi_slave(dev);
> +
> + kfree(slave);
> +}
> +
> static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id)
> {
> - /* todo: initialise slave device, perform engine scan */
> + struct fsi_slave *slave;
> + uint32_t chip_id;
> + uint8_t crc;
> + int rc;
> +
> + /* Currently, we only support single slaves on a link, and use the
> + * full 23-bit address range
> + */
> + if (id != 0)
> + return -EINVAL;
> +
> + rc = fsi_master_read(master, link, id, 0, &chip_id, sizeof(chip_id));
> + if (rc) {
> + dev_warn(&master->dev, "can't read slave %02x:%02x %d\n",
> + link, id, rc);
When I boot a system with this driver loaded, I get his warning:
[ 9.740000] usbhid: USB HID core driver
[ 9.840000] fsi0: can't read slave 00:00 -5
[ 9.840000] NET: Registered protocol family 10
Two things:
There's a space in front of "fsi0".
This warning isn't useful at that point. The slave is not readable as
the FSI master is not present (the P9 hasn't been turned on). Can we
avoid printing the warning at boot?
Cheers,
Joel
> + return -ENODEV;
> + }
> + chip_id = be32_to_cpu(chip_id);
> +
> + crc = fsi_crc4(0, chip_id, 32);
> + if (crc) {
> + dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n",
> + link, id);
> + return -EIO;
> + }
> +
> + dev_info(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n",
> + chip_id, master->idx, link, id);
> +
> + /* We can communicate with a slave; create the slave device and
> + * register.
> + */
> + slave = kzalloc(sizeof(*slave), GFP_KERNEL);
> + if (!slave)
> + return -ENOMEM;
> +
> + slave->master = master;
> + slave->dev.parent = &master->dev;
> + slave->dev.release = fsi_slave_release;
> + slave->link = link;
> + slave->id = id;
> + slave->size = FSI_SLAVE_SIZE_23b;
> +
> + dev_set_name(&slave->dev, "slave@%02x:%02x", link, id);
> + rc = device_register(&slave->dev);
> + if (rc < 0) {
> + dev_warn(&master->dev, "failed to create slave device: %d\n",
> + rc);
> + put_device(&slave->dev);
> + return rc;
> + }
> +
> + /* todo: perform engine scan */
>
> - return -ENODEV;
> + return rc;
> }
>
> /* FSI master support */
> --
> 1.8.2.2
>
--
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 RFC] Documentation/devicetree: Add specification for FSI busses
From: Joel Stanley @ 2017-05-02 6:19 UTC (permalink / raw)
To: Jeremy Kerr; +Cc: devicetree, OpenBMC Maillist
In-Reply-To: <1493704508-27119-1-git-send-email-jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
On Tue, May 2, 2017 at 3:25 PM, Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org> wrote:
> This change introduces a proposed layout for describing FSI busses in
> the device tree. While the bus is probe-able, we'd still like a method
> of describing subordinate (eg i2c) busses that are behind FSI devices.
>
> The FSI core will be responsible for matching probed slaves & engines to
> their device tree nodes, so the FSI device drivers' probe() functions
> will be passed a struct device with the appropriate of_node populated
> where a matching DT node is found.
>
> RFC at this stage, so I'd welcome any feedback.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
Looks good to me Jeremy. One small question on the description, but
please add Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> to future
revisions.
> ---
> Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
> 1 file changed, 144 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
> new file mode 100644
> index 0000000..b5e85c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi.txt
> @@ -0,0 +1,144 @@
> +FSI bus & engine generic device tree bindings
> +=============================================
> +
> +The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
> +engines within those slaves. However, we have a facility to match devicetree
> +nodes to probed engines. This allows for fsi engines to expose non-probeable
> +busses, which are then exposed by the device tree. For example, a FSI engine
A nit: Can we can expose any device as part of the engine, not just a bus?
> +that is an I2C master - the I2C bus can be described by the device tree under
> +the engine's device tree node.
Cheers,
Joel
--
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 3/3] USB3/DWC3: Enable undefined length INCR burst type
From: Jerry Huang @ 2017-05-02 6:13 UTC (permalink / raw)
To: Felipe Balbi, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
catalin.marinas-5wv7dgnIgG8@public.gmane.org
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Rajesh Bhagat
In-Reply-To: <87tw71xthj.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> -----Original Message-----
> From: Felipe Balbi [mailto:balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Friday, March 10, 2017 7:27 PM
> To: Jerry Huang <jerry.huang-3arQi8VN3Tc@public.gmane.org>; robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org;
> mark.rutland-5wv7dgnIgG8@public.gmane.org; catalin.marinas-5wv7dgnIgG8@public.gmane.org
> Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; Rajesh
> Bhagat <rajesh.bhagat-3arQi8VN3Tc@public.gmane.org>
> Subject: RE: [PATCH v4 3/3] USB3/DWC3: Enable undefined length INCR burst
> type
>
>
> Hi,
>
> Jerry Huang <jerry.huang-3arQi8VN3Tc@public.gmane.org> writes:
> >> >> --
> >> >> 1.7.9.5
> >> > Hi, Balbi and all guys,
> >> > Any comment for these patches? Can they be accepted?
> >>
> >> Rob had comments which you didn't reply yet. I cannot take this
> >> patchset yet ;-)
> >>
> > Balbi,
> >
> > I look into his mail again, which was based v3, and I replied it.
> >
> > He had different understanding for undefined length burst mode.
> >
> > It seems he think for this mode, just setting bit[0] (INCRBrstEna) and
> > don't need to set other field.
> >
> > However, according to the DWC USB3.0 controller databook, when it is
> > undefined length INCR burst mode, we still need to set one max burst
> > type, such as INCR8, which means controller will use any length less
> > than or equal to this INCR8.
>
> Rob, do you agree with the patch now?
>
> --
> balbi
Hi, Balbi,
Any comment for these patches? Or any chance to merge them?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 RFC] Documentation/devicetree: Add specification for FSI busses
From: Jeremy Kerr @ 2017-05-02 5:55 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
This change introduces a proposed layout for describing FSI busses in
the device tree. While the bus is probe-able, we'd still like a method
of describing subordinate (eg i2c) busses that are behind FSI devices.
The FSI core will be responsible for matching probed slaves & engines to
their device tree nodes, so the FSI device drivers' probe() functions
will be passed a struct device with the appropriate of_node populated
where a matching DT node is found.
RFC at this stage, so I'd welcome any feedback.
Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/fsi/fsi.txt | 144 ++++++++++++++++++++++++++
1 file changed, 144 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fsi/fsi.txt
diff --git a/Documentation/devicetree/bindings/fsi/fsi.txt b/Documentation/devicetree/bindings/fsi/fsi.txt
new file mode 100644
index 0000000..b5e85c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/fsi.txt
@@ -0,0 +1,144 @@
+FSI bus & engine generic device tree bindings
+=============================================
+
+The FSI bus is probe-able, so Linux is able to enumerate FSI slaves, and
+engines within those slaves. However, we have a facility to match devicetree
+nodes to probed engines. This allows for fsi engines to expose non-probeable
+busses, which are then exposed by the device tree. For example, a FSI engine
+that is an I2C master - the I2C bus can be described by the device tree under
+the engine's device tree node.
+
+FSI masters may require their own DT nodes (to describe the master HW itself);
+that requirement is defined by the master's implementation, and is described by
+the fsi-master-* binding specifications.
+
+Under the masters' nodes, we can describe the bus topology using nodes to
+represent the FSI slaves and their slave engines. As a basic outline:
+
+ fsi-master {
+ /* top-level of FSI bus topology, bound to a FSI master driver and
+ * exposes a FSI bus */
+
+ fsi-slave@<link,id> {
+ /* this node defines the FSI slave device, and is handled
+ * entirely with FSI core code */
+
+ fsi-slave-engine@<addr> {
+ /* this node defines the engine endpoint & address range, which
+ * is bound to the relevant fsi device driver */
+ ...
+ };
+
+ fsi-slave-engine@<addr> {
+ ...
+ };
+
+ };
+ };
+
+Note that since the bus is probe-able, some (or all) of the topology may
+not be described; this binding only provides an optional facility for
+adding subordinate device tree nodes as children of FSI engines.
+
+FSI masters
+-----------
+
+FSI master nodes declare themselves as such with the "fsi-master" compatible
+value. It's likely that an implementation-specific compatible value will
+be needed as well, for example:
+
+ compatible = "fsi-master-gpio", "fsi-master";
+
+Since the master nodes describe the top-level of the FSI topology, they also
+need to declare the FSI-standard addressing scheme. This requires two cells for
+addresses (link index and slave ID), and no size:
+
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+FSI slaves
+----------
+
+Slaves are identified by a (link-index, slave-id) pair, so require two cells
+for an address identifier. Since these are not a range, no size cells are
+required. For an example, a slave on link 1, with ID 2, could be represented
+as:
+
+ cfam@1,2 {
+ reg = <1 2>;
+ [...];
+ }
+
+Each slave provides an address-space, under which the engines are accessible.
+That address space has a maximum of 23 bits, so we use one cell to represent
+addresses and sizes in the slave address space:
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+
+FSI engines (devices)
+---------------------
+
+Engines are identified by their address under the slaves' address spaces. We
+use a single cell for address and size. Engine nodes represent the endpoint
+FSI device, and are passed to those FSI device drivers' ->probe() functions.
+
+For example, for a slave using a single 0x400-byte page starting at address
+0xc00:
+
+ engine@c00 {
+ reg = <0xc00 0x400>;
+ };
+
+
+Full example
+------------
+
+Here's an example that illustrates:
+ - a FSI master
+ - connected to a FSI slave
+ - that contains an engine that is an I2C master
+ - connected to an I2C EEPROM
+
+The FSI master may be connected to additional slaves, and slaves may have
+additional engines, but they don't necessarily need to be describe in the
+device tree if no extra platform information is required.
+
+ /* The GPIO-based FSI master node, describing the top level of the
+ * FSI bus
+ */
+ gpio-fsi {
+ compatible = "fsi-master-gpio", "fsi-master";
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ /* A FSI slave (aka. CFAM) at link 0, ID 0. */
+ cfam@0,0 {
+ reg = <0 0>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ /* FSI engine at 0xc00, using a single page. In this example,
+ * it's an I2C master controller, so subnodes describe the
+ * I2C bus.
+ */
+ i2c-controller@c00 {
+ reg = <0xc00 0x400>;
+
+ /* Engine-specific data. In this case, we're describing an
+ * I2C bus, so we're conforming to the generic I2C binding
+ */
+ compatible = "ibm,fsi-i2c";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ /* I2C endpoint device: an Atmel EEPROM */
+ eeprom@50 {
+ compatible = "atmel,24c256";
+ reg = <0x50>;
+ pagesize = <64>;
+ };
+ };
+ };
+ };
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [v6,10/23] drivers/fsi: Add device read/write/peek API
From: Brad Bishop @ 2017-05-02 5:25 UTC (permalink / raw)
To: Christopher Bostic
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mingo-H+wXaHxf7aLQT0dZR+AlfA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
andrew-zrmu5oMJ5Fs, alistair-Y4h6yKqj69EXC2x5gXVKYQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jeremy Kerr,
Edward A . James, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
joel-U3u1mxZcP9KHXe+LvDLADg
In-Reply-To: <20170410194706.64280-11-cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> On Apr 10, 2017, at 3:46 PM, Christopher Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
>
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change introduces the fsi device API: simple read, write and peek
> accessors for the devices' address spaces.
>
> Includes contributions from Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> and Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>.
>
> Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Chris Bostic <cbostic-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> ---
> drivers/fsi/fsi-core.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/fsi.h | 7 ++++++-
> 2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index a8faa89..4da0b030 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -32,6 +32,8 @@
> #define FSI_SLAVE_CONF_CRC_MASK 0x0000000f
> #define FSI_SLAVE_CONF_DATA_BITS 28
>
> +#define FSI_PEEK_BASE 0x410
> +
> static const int engine_page_size = 0x400;
>
> #define FSI_SLAVE_BASE 0x800
> @@ -73,9 +75,40 @@ static int fsi_master_read(struct fsi_master *master, int link,
> uint8_t slave_id, uint32_t addr, void *val, size_t size);
> static int fsi_master_write(struct fsi_master *master, int link,
> uint8_t slave_id, uint32_t addr, const void *val, size_t size);
> +static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
> + void *val, size_t size);
> +static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,
> + const void *val, size_t size);
>
> /* FSI endpoint-device support */
>
> +int fsi_device_read(struct fsi_device *dev, uint32_t addr, void *val,
> + size_t size)
> +{
> + if (addr > dev->size || size > dev->size || addr > dev->size - size)
> + return -EINVAL;
> +
> + return fsi_slave_read(dev->slave, dev->addr + addr, val, size);
> +}
> +EXPORT_SYMBOL_GPL(fsi_device_read);
> +
> +int fsi_device_write(struct fsi_device *dev, uint32_t addr, const void *val,
> + size_t size)
> +{
> + if (addr > dev->size || size > dev->size || addr > dev->size - size)
> + return -EINVAL;
> +
> + return fsi_slave_write(dev->slave, dev->addr + addr, val, size);
> +}
> +EXPORT_SYMBOL_GPL(fsi_device_write);
> +
> +int fsi_device_peek(struct fsi_device *dev, void *val)
> +{
> + uint32_t addr = FSI_PEEK_BASE + ((dev->unit - 2) * sizeof(uint32_t));
> +
> + return fsi_slave_read(dev->slave, addr, val, sizeof(uint32_t));
> +}
> +
> static void fsi_device_release(struct device *_device)
> {
> struct fsi_device *device = to_fsi_dev(_device);
> diff --git a/include/linux/fsi.h b/include/linux/fsi.h
> index efa55ba..66bce48 100644
> --- a/include/linux/fsi.h
> +++ b/include/linux/fsi.h
> @@ -27,6 +27,12 @@ struct fsi_device {
> uint32_t size;
> };
>
> +extern int fsi_device_read(struct fsi_device *dev, uint32_t addr,
> + void *val, size_t size);
> +extern int fsi_device_write(struct fsi_device *dev, uint32_t addr,
> + const void *val, size_t size);
> +extern int fsi_device_peek(struct fsi_device *dev, void *val);
> +
> struct fsi_device_id {
> u8 engine_type;
> u8 version;
> @@ -40,7 +46,6 @@ struct fsi_device_id {
> #define FSI_DEVICE_VERSIONED(t, v) \
> .engine_type = (t), .version = (v),
>
> -
> struct fsi_driver {
> struct device_driver drv;
> const struct fsi_device_id *id_table;
I wrote a driver using this API. It seems to be suitable enough.
Acked-by: Brad Bishop <bradleyb-r5pk2Da7Bxt8sGd51Jp2sdBPR1lH4CV8@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 4/4] of: detect invalid phandle in overlay
From: frowand.list @ 2017-05-02 2:46 UTC (permalink / raw)
To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel
In-Reply-To: <1493693164-22826-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
Overlays are not allowed to modify phandle values of previously existing
nodes because there is no information available to allow fixup up
properties that use the previously existing phandle.
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
drivers/of/overlay.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index ca0b85f5deb1..20ab49d2f7a4 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -130,6 +130,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
/* NOTE: Multiple mods of created nodes not supported */
tchild = of_get_child_by_name(target, cname);
if (tchild != NULL) {
+ /* new overlay phandle value conflicts with existing value */
+ if (child->phandle)
+ return -EINVAL;
+
/* apply overlay recursively */
ret = of_overlay_apply_one(ov, tchild, child);
of_node_put(tchild);
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* [PATCH v4 3/4] of: be consistent in form of file mode
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-05-02 2:46 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493693164-22826-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
checkpatch whined about using S_IRUGO instead of octal equivalent
when adding phandle sysfs code, so used octal in that patch.
Change other instances of the S_* constants in the same file to
the octal form.
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8a0cf9003cf8..8a7ca2a49ba8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -168,7 +168,7 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
sysfs_bin_attr_init(&pp->attr);
pp->attr.attr.name = safe_name(&np->kobj, pp->name);
- pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
+ pp->attr.attr.mode = secure ? 0400 : 0444;
pp->attr.size = secure ? 0 : pp->length;
pp->attr.read = of_node_property_read;
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 2/4] of: make __of_attach_node() static
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-05-02 2:46 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493693164-22826-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
__of_attach_node() is not used outside of drivers/of/dynamic.c. Make
it static and remove it from drivers/of/of_private.h.
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/dynamic.c | 2 +-
drivers/of/of_private.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 59545b8fbf46..0b9cf6d5914c 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -216,7 +216,7 @@ int of_property_notify(int action, struct device_node *np,
return of_reconfig_notify(action, &pr);
}
-void __of_attach_node(struct device_node *np)
+static void __of_attach_node(struct device_node *np)
{
np->child = NULL;
np->sibling = np->parent->child;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 33f11a5384f3..b1ff70e1fdda 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -79,7 +79,6 @@ extern int __of_update_property(struct device_node *np,
extern void __of_update_property_sysfs(struct device_node *np,
struct property *newprop, struct property *oldprop);
-extern void __of_attach_node(struct device_node *np);
extern int __of_attach_node_sysfs(struct device_node *np);
extern void __of_detach_node(struct device_node *np);
extern void __of_detach_node_sysfs(struct device_node *np);
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 1/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-05-02 2:46 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493693164-22826-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
Remove "phandle", "linux,phandle", and "ibm,phandle" properties from
the internal device tree. The phandle will still be in the struct
device_node phandle field.
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
attached to the live tree. Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
"ibm,phandle" properties will no longer appear in /proc/device-tree (they
will appear as "phandle").
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 48 ++++++++++++++++++++++++++++++++++++++++---
drivers/of/dynamic.c | 54 +++++++++++++++++++++++++++++++++++++------------
drivers/of/fdt.c | 40 +++++++++++++++++++++---------------
drivers/of/of_private.h | 1 +
drivers/of/overlay.c | 4 +---
drivers/of/resolver.c | 23 +--------------------
include/linux/of.h | 1 +
7 files changed, 114 insertions(+), 57 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2d..8a0cf9003cf8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
}
+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t offset, size_t count)
+{
+ phandle phandle;
+ struct device_node *np;
+
+ np = container_of(bin_attr, struct device_node, attr_phandle);
+ phandle = cpu_to_be32(np->phandle);
+ return memory_read_from_buffer(buf, count, &offset, &phandle,
+ sizeof(phandle));
+}
+
/* always return newly allocated name, caller must free after use */
static const char *safe_name(struct kobject *kobj, const char *orig_name)
{
@@ -164,6 +177,35 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
return rc;
}
+/*
+ * In the imported device tree (fdt), phandle is a property. In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+ int rc;
+
+ if (!IS_ENABLED(CONFIG_SYSFS))
+ return 0;
+
+ if (!of_kset || !of_node_is_attached(np))
+ return 0;
+
+ if (!np->phandle || np->phandle == 0xffffffff)
+ return 0;
+
+ sysfs_bin_attr_init(&np->attr_phandle);
+ np->attr_phandle.attr.name = "phandle";
+ np->attr_phandle.attr.mode = 0444;
+ np->attr_phandle.size = sizeof(np->phandle);
+ np->attr_phandle.read = of_node_phandle_read;
+
+ rc = sysfs_create_bin_file(&np->kobj, &np->attr_phandle);
+ WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+ return rc;
+}
+
int __of_attach_node_sysfs(struct device_node *np)
{
const char *name;
@@ -193,6 +235,8 @@ int __of_attach_node_sysfs(struct device_node *np)
if (rc)
return rc;
+ __of_add_phandle_sysfs(np);
+
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
@@ -2097,9 +2141,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
int id, len;
/* Skip those we do not want to proceed */
- if (!strcmp(pp->name, "name") ||
- !strcmp(pp->name, "phandle") ||
- !strcmp(pp->name, "linux,phandle"))
+ if (!strcmp(pp->name, "name"))
continue;
np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..59545b8fbf46 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,
void __of_attach_node(struct device_node *np)
{
- const __be32 *phandle;
- int sz;
-
- np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
- np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
-
- phandle = __of_get_property(np, "phandle", &sz);
- if (!phandle)
- phandle = __of_get_property(np, "linux,phandle", &sz);
- if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
- phandle = __of_get_property(np, "ibm,phandle", &sz);
- np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
-
np->child = NULL;
np->sibling = np->parent->child;
np->parent->child = np;
@@ -243,11 +230,47 @@ void __of_attach_node(struct device_node *np)
int of_attach_node(struct device_node *np)
{
struct of_reconfig_data rd;
+ struct property *prev;
+ struct property *prop;
+ struct property *save_next;
unsigned long flags;
+ const __be32 *phandle;
+ int sz;
memset(&rd, 0, sizeof(rd));
rd.dn = np;
+ np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
+ np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
+
+ phandle = __of_get_property(np, "phandle", &sz);
+ if (!phandle)
+ phandle = __of_get_property(np, "linux,phandle", &sz);
+ if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
+ phandle = __of_get_property(np, "ibm,phandle", &sz);
+ np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
+
+ /* remove phandle properties from node */
+ prev = NULL;
+ for (prop = np->properties; prop != NULL; ) {
+ save_next = prop->next;
+ if (!strcmp(prop->name, "phandle") ||
+ !strcmp(prop->name, "ibm,phandle") ||
+ !strcmp(prop->name, "linux,phandle")) {
+ if (prev)
+ prev->next = prop->next;
+ else
+ np->properties = prop->next;
+ prop->next = np->deadprops;
+ np->deadprops = prop;
+ } else {
+ prev = prop;
+ }
+ prop = save_next;
+ }
+
+ __of_add_phandle_sysfs(np);
+
mutex_lock(&of_mutex);
raw_spin_lock_irqsave(&devtree_lock, flags);
__of_attach_node(np);
@@ -429,6 +452,7 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
/* Iterate over and duplicate all properties */
if (np) {
struct property *pp, *new_pp;
+ node->phandle = np->phandle;
for_each_property_of_node(np, pp) {
new_pp = __of_prop_dup(pp, GFP_KERNEL);
if (!new_pp)
@@ -441,6 +465,10 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
}
}
}
+
+ node->name = __of_get_property(node, "name", NULL) ? : "<NULL>";
+ node->type = __of_get_property(node, "device_type", NULL) ? : "<NULL>";
+
return node;
err_prop:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..270f31b0c259 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -181,6 +181,8 @@ static void populate_properties(const void *blob,
const __be32 *val;
const char *pname;
u32 sz;
+ int prop_is_phandle = 0;
+ int prop_is_ibm_phandle = 0;
val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
if (!val) {
@@ -196,11 +198,6 @@ static void populate_properties(const void *blob,
if (!strcmp(pname, "name"))
has_name = true;
- pp = unflatten_dt_alloc(mem, sizeof(struct property),
- __alignof__(struct property));
- if (dryrun)
- continue;
-
/* We accept flattened tree phandles either in
* ePAPR-style "phandle" properties, or the
* legacy "linux,phandle" properties. If both
@@ -208,23 +205,34 @@ static void populate_properties(const void *blob,
* will get weird. Don't do that.
*/
if (!strcmp(pname, "phandle") ||
- !strcmp(pname, "linux,phandle")) {
- if (!np->phandle)
- np->phandle = be32_to_cpup(val);
- }
+ !strcmp(pname, "linux,phandle"))
+ prop_is_phandle = 1;
/* And we process the "ibm,phandle" property
* used in pSeries dynamic device tree
* stuff
*/
- if (!strcmp(pname, "ibm,phandle"))
- np->phandle = be32_to_cpup(val);
+ if (!strcmp(pname, "ibm,phandle")) {
+ prop_is_phandle = 1;
+ prop_is_ibm_phandle = 1;
+ }
+
+ if (!prop_is_phandle)
+ pp = unflatten_dt_alloc(mem, sizeof(struct property),
+ __alignof__(struct property));
- pp->name = (char *)pname;
- pp->length = sz;
- pp->value = (__be32 *)val;
- *pprev = pp;
- pprev = &pp->next;
+ if (dryrun)
+ continue;
+
+ if (!prop_is_phandle) {
+ pp->name = (char *)pname;
+ pp->length = sz;
+ pp->value = (__be32 *)val;
+ *pprev = pp;
+ pprev = &pp->next;
+ } else if (prop_is_ibm_phandle || !np->phandle) {
+ np->phandle = be32_to_cpup(val);
+ }
}
/* With version 0x10 we may not have the name property,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..33f11a5384f3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -47,6 +47,7 @@ extern int of_property_notify(int action, struct device_node *np,
extern void of_node_release(struct kobject *kobj);
extern int __of_changeset_apply(struct of_changeset *ocs);
extern int __of_changeset_revert(struct of_changeset *ocs);
+extern int __of_add_phandle_sysfs(struct device_node *np);
#else /* CONFIG_OF_DYNAMIC */
static inline int of_property_notify(int action, struct device_node *np,
struct property *prop, struct property *old_prop)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7827786718d8..ca0b85f5deb1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -101,9 +101,7 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
tprop = of_find_property(target, prop->name, NULL);
/* special properties are not meant to be updated (silent NOP) */
- if (of_prop_cmp(prop->name, "name") == 0 ||
- of_prop_cmp(prop->name, "phandle") == 0 ||
- of_prop_cmp(prop->name, "linux,phandle") == 0)
+ if (!of_prop_cmp(prop->name, "name"))
return 0;
propn = __of_prop_dup(prop, GFP_KERNEL);
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 7ae9863cb0a4..624cbaeae0a4 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -71,30 +71,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
int phandle_delta)
{
struct device_node *child;
- struct property *prop;
- phandle phandle;
/* adjust node's phandle in node */
if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
overlay->phandle += phandle_delta;
- /* copy adjusted phandle into *phandle properties */
- for_each_property_of_node(overlay, prop) {
-
- if (of_prop_cmp(prop->name, "phandle") &&
- of_prop_cmp(prop->name, "linux,phandle"))
- continue;
-
- if (prop->length < 4)
- continue;
-
- phandle = be32_to_cpup(prop->value);
- if (phandle == OF_PHANDLE_ILLEGAL)
- continue;
-
- *(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
- }
-
for_each_child_of_node(overlay, child)
adjust_overlay_phandles(child, phandle_delta);
}
@@ -197,9 +178,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
for_each_property_of_node(local_fixups, prop_fix) {
/* skip properties added automatically */
- if (!of_prop_cmp(prop_fix->name, "name") ||
- !of_prop_cmp(prop_fix->name, "phandle") ||
- !of_prop_cmp(prop_fix->name, "linux,phandle"))
+ if (!of_prop_cmp(prop_fix->name, "name"))
continue;
if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323de0f3..4e86e05853f3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -61,6 +61,7 @@ struct device_node {
struct kobject kobj;
unsigned long _flags;
void *data;
+ struct bin_attribute attr_phandle;
#if defined(CONFIG_SPARC)
const char *path_component_name;
unsigned int unique_id;
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 0/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-05-02 2:46 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
Remove "phandle" and "linux,phandle" properties from the internal
device tree. The phandle will still be in the struct device_node
phandle field.
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
Patch 1 is the phandle related changes.
Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.
Changes from v3:
- patch 1: fix incorrect variable name in __of_add_phandle_sysfs().
Problem was reported by the kbuild test robot
Changes from v2:
- patch 1: Remove check in __of_add_phandle_sysfs() that would not
add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
Changes from v1:
- Remove phandle properties in of_attach_node(), before attaching
the node to the live tree.
- Add the phandle sysfs entry for the node in of_attach_node().
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
Frank Rowand (4):
of: remove *phandle properties from expanded device tree
of: make __of_attach_node() static
of: be consistent in form of file mode
of: detect invalid phandle in overlay
drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
drivers/of/of_private.h | 2 +-
drivers/of/overlay.c | 8 ++++---
drivers/of/resolver.c | 23 +-------------------
include/linux/of.h | 1 +
7 files changed, 120 insertions(+), 60 deletions(-)
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 7/9] power: Adds support for Smart Battery System Manager
From: Phil Reid @ 2017-05-02 2:20 UTC (permalink / raw)
To: Sebastian Reichel
Cc: wsa, robh+dt, mark.rutland, peda, linux-i2c, devicetree, linux-pm,
Karl-Heinz Schneider
In-Reply-To: <20170501134136.u2tebw77xcirvaqx@earth>
G'day Sebastian,
Thanks for the feedback. Next version coming soon.
In regards the le to cpu conversion.
grepping the supply folder finds a couple of other suspicious cases.
sbs-battery & the bq24735-cahrger do similar things.
thoughts on if they need to be corrected?
On 1/05/2017 21:41, Sebastian Reichel wrote:
> Hi,
>
> On Mon, May 01, 2017 at 04:49:57PM +0800, Phil Reid wrote:
>> From: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
>>
>> This patch adds support for Smart Battery System Manager.
>> A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of
>> communicating up to four I2C smart battery devices. All smart battery
>> devices are listening at address 0x0b, so the SBSM muliplexes between
>> them. The driver makes use of the I2C-Mux framework to allow smart
>> batteries to be bound via device tree, i.e. the sbs-battery driver.
>>
>> Via sysfs interface the online state and charge type are presented. If
>> the driver is bound as ltc1760 (an implementation of a Dual Smart Battery
>> System Manager) the charge type can also be changed from trickle to fast.
>>
>> Tested-by: Phil Reid <preid@electromag.com.au>
>> Reviewed-by: Phil Reid <preid@electromag.com.au>
>> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>> drivers/power/supply/Kconfig | 13 ++
>> drivers/power/supply/Makefile | 1 +
>> drivers/power/supply/sbs-manager.c | 325 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 339 insertions(+)
>> create mode 100644 drivers/power/supply/sbs-manager.c
>>
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index da54ac8..8aa5324 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -533,4 +533,17 @@ config AXP20X_POWER
>> This driver provides support for the power supply features of
>> AXP20x PMIC.
>>
>> +config MANAGER_SBS
>> + tristate "Smart Battery System Manager"
>> + depends on I2C && I2C_MUX
>> + help
>> + Say Y here to include support for Smart Battery System Manager
>> + ICs. The driver reports online and charging status via sysfs.
>> + It presents itself also as I2C mux which allows to bind
>> + smart battery driver to its ports.
>> + Supported is for example LTC1760.
>> +
>> + This driver can also be built as a module. If so, the module will be
>> + called sbs-manager.
>> +
>
> Let's move add this directly next to CHARGER_SBS.
>
>> endif # POWER_SUPPLY
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index 3789a2c..4f53c98 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
>> obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
>> obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
>> obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
>> +obj-$(CONFIG_MANAGER_SBS) += sbs-manager.o
>
> same here.
>
>> diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
>> new file mode 100644
>> index 0000000..adf9e41
>> --- /dev/null
>> +++ b/drivers/power/supply/sbs-manager.c
>> @@ -0,0 +1,325 @@
>> +/*
>> + * Driver for SBS compliant Smart Battery System Managers
>> + *
>> + * The device communicates via i2c at address 0x0a and multiplexes access to up
>> + * to four smart batteries at address 0x0b.
>> + *
>> + * Via sysfs interface the online state and charge type are presented.
>> + *
>> + * Datasheet SBSM: http://sbs-forum.org/specs/sbsm100b.pdf
>> + * Datasheet LTC1760: http://cds.linear.com/docs/en/datasheet/1760fb.pdf
>> + *
>> + * Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/i2c-mux.h>
>> +#include <linux/power_supply.h>
>> +
>> +#define SBSM_MAX_BATS 4
>> +#define SBSM_RETRY_CNT 3
>> +
>> +/* registers addresses */
>> +#define SBSM_CMD_BATSYSSTATE 0x01
>> +#define SBSM_CMD_BATSYSSTATECONT 0x02
>> +#define SBSM_CMD_BATSYSINFO 0x04
>> +#define SBSM_CMD_LTC 0x3c
>> +
>> +struct sbsm_data {
>> + struct i2c_client *client;
>> + struct i2c_mux_core *muxc;
>> +
>> + struct power_supply *psy;
>> +
>> + int cur_chan; /* currently selected channel */
>
> u8?
>
>> + bool is_ltc1760; /* special capabilities */
>> +};
>> +
>> +static enum power_supply_property sbsm_props[] = {
>> + POWER_SUPPLY_PROP_ONLINE,
>> + POWER_SUPPLY_PROP_CHARGE_TYPE,
>> +};
>> +
>> +static int sbsm_read_word(struct i2c_client *client, u8 address)
>> +{
>> + int reg, retries = SBSM_RETRY_CNT;
>> +
>> + while (retries > 0) {
>> + reg = i2c_smbus_read_word_data(client, address);
>> + if (reg >= 0)
>> + break;
>> + --retries;
>> + }
>
> for (retries = SBSM_RETRY_CNT; retries > 0; retries--)
>
>> +
>> + if (reg < 0) {
>> + dev_err(&client->dev, "failed to read register %i\n",
>> + (int)address);
>
> dev_err(&client->dev, "failed to read register 0x%02x\n", address);
>
>> + return reg;
>> + }
>> +
>> + return le16_to_cpu(reg);
>
> This is already done by i2c_smbus_read_word_data() and doing it
> again will result in incorrect values on big endian architectures.
>
>> +}
>> +
>> +static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word)
>> +{
>> + int ret, retries = SBSM_RETRY_CNT;
>> +
>> + word = cpu_to_le16(word);
>
> Same as for read_word: This is already done by the i2c-core.
>
>> + while (retries > 0) {
>> + ret = i2c_smbus_write_word_data(client, address, word);
>> + if (ret >= 0)
>> + break;
>> + --retries;
>> + }
>
> for (retries = SBSM_RETRY_CNT; retries > 0; retries--)
>
>> + if (ret < 0)
>> + dev_err(&client->dev, "failed to write to register %i\n",
>> + (int)address);
>
> dev_err(&client->dev, "failed to read register 0x%02x\n", address);
>
>> +
>> + return ret;
>> +}
>> +
>> +static int sbsm_get_property(struct power_supply *psy,
>> + enum power_supply_property psp,
>> + union power_supply_propval *val)
>> +{
>> + struct sbsm_data *data = power_supply_get_drvdata(psy);
>> + int regval = 0;
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_ONLINE:
>> + regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT);
>> + if (regval < 0)
>> + return regval;
>> + val->intval = !!(regval & 0x1);
>> + break;
>> +
>> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
>> + regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE);
>> + if (regval < 0)
>> + return regval;
>> +
>> + if ((regval & 0x00f0) == 0) {
>> + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
>> + return 0;
>> + }
>> + val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
>> +
>> + if (data->is_ltc1760) {
>> + /* charge mode fast if turbo is active */
>> + regval = sbsm_read_word(data->client, SBSM_CMD_LTC);
>> + if (regval < 0)
>> + return regval;
>> + else if (regval & 0x80)
>> + val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
>> + }
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int sbsm_prop_is_writeable(struct power_supply *psy,
>> + enum power_supply_property psp)
>> +{
>> + struct sbsm_data *data = power_supply_get_drvdata(psy);
>> +
>> + return (psp == POWER_SUPPLY_PROP_CHARGE_TYPE) && data->is_ltc1760;
>> +}
>> +
>> +static int sbsm_set_property(struct power_supply *psy,
>> + enum power_supply_property psp,
>> + const union power_supply_propval *val)
>> +{
>> + struct sbsm_data *data = power_supply_get_drvdata(psy);
>> + int ret = -EINVAL;
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
>> + /* write 1 to TURBO if type fast is given */
>> + if (data->is_ltc1760) {
>> + u16 regval = val->intval ==
>> + POWER_SUPPLY_CHARGE_TYPE_FAST ? (0x1 << 7) : 0;
>> + ret = sbsm_write_word(data->client, SBSM_CMD_LTC,
>> + regval);
>> + }
>
> That's not nicely indented. Try doing it this way to
> reduce indention:
>
> if (data->is_ltc1760)
> break;
> u16 regval = ...
>
>> + break;
>> +
>> + default:
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * Switch to battery
>> + * Parameter chan is directly the content of SMB_BAT* nibble
>> + */
>> +static int sbsm_select(struct i2c_mux_core *muxc, u32 chan)
>> +{
>> + struct sbsm_data *data = i2c_mux_priv(muxc);
>> + int ret = 0;
>> + u16 reg;
>> +
>> + if (data->cur_chan == chan)
>> + return ret;
>> +
>> + /* chan goes from 1 ... 4 */
>> + reg = 1 << (11 + chan);
>> + ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, reg);
>> + if (ret)
>> + dev_err(&data->client->dev, "Failed to select channel %i\n",
>> + chan);
>
> Add
>
> struct device *dev = &data->client->dev;
>
> at the beginning of the function and use it here to avoid line
> break.
>
>> + else
>> + data->cur_chan = chan;
>> +
>> + return ret;
>> +}
>> +
>> +#if defined(CONFIG_OF)
>> +
>> +#include <linux/of_device.h>
>
> Move include to top of file. You can include it unconditionally.
>
>> +static const struct of_device_id sbsm_dt_ids[] = {
>> + { .compatible = "sbs,sbs-manager" },
>> + { .compatible = "lltc,ltc1760" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, sbsm_dt_ids);
>
> This can go next to the other MODULE_DEVICE_TABLE.
>
>> +#endif
>> +
>> +static const struct power_supply_desc sbsm_default_psy_desc = {
>> + .type = POWER_SUPPLY_TYPE_MAINS,
>> + .properties = sbsm_props,
>> + .num_properties = ARRAY_SIZE(sbsm_props),
>> + .get_property = &sbsm_get_property,
>> + .set_property = &sbsm_set_property,
>> + .property_is_writeable = &sbsm_prop_is_writeable,
>> +};
>> +
>> +static int sbsm_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
>> + struct sbsm_data *data;
>> + struct device *dev = &client->dev;
>> + struct power_supply_desc *psy_desc;
>> + struct power_supply_config psy_cfg = {};
>> + int ret = 0, i, supported_bats;
>> +
>> + /* Device listens only at address 0x0a */
>> + if (client->addr != 0x0a)
>> + return -ENODEV;
>
> I guess EINVAL makes more sense?
>
>> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
>> + return -EPFNOSUPPORT;
>> +
>> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + i2c_set_clientdata(client, data);
>> +
>> + data->client = client;
>> + data->is_ltc1760 = !!strstr(id->name, "ltc1760");
>> +
>> + ret = sbsm_read_word(client, SBSM_CMD_BATSYSINFO);
>> + if (ret < 0)
>> + return ret;
>> + supported_bats = le16_to_cpu(ret) & 0xf;
>
> How often do you want to convert endianess? :)
> Drop this.
>
>> + data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
>> + I2C_MUX_LOCKED, &sbsm_select, NULL);
>> + if (!data->muxc) {
>> + dev_err(dev, "failed to alloc i2c mux\n");
>> + ret = -ENOMEM;
>> + goto err_mux_alloc;
>> + }
>> + data->muxc->priv = data;
>> +
>> + /* register muxed i2c channels. One for each supported battery */
>> + for (i = 0; i < SBSM_MAX_BATS; ++i) {
>> + if ((1 << i) & supported_bats) {
>
> This can be written more readable as
>
> if (supported_bats & BIT(i))
>
>> + ret = i2c_mux_add_adapter(data->muxc, 0, i + 1, 0);
>> + if (ret) {
>> + dev_err(dev,
>> + "failed to register i2c mux channel %d\n",
>> + i + 1);
>> + goto err_mux_register;
>> + }
>> + }
>> + }
>> +
>> + psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc,
>> + sizeof(struct power_supply_desc),
>> + GFP_KERNEL);
>> + if (!psy_desc) {
>> + ret = -ENOMEM;
>> + goto err_psy;
>> + }
>> +
>> + psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s",
>> + dev_name(&client->dev));
>> + if (!psy_desc->name) {
>> + ret = -ENOMEM;
>> + goto err_psy;
>> + }
>> +
>> + psy_cfg.drv_data = data;
>
> Please add:
>
> psy_cfg->of_node = dev->of_node;
>
>> + data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
>> + if (IS_ERR(data->psy)) {
>> + ret = PTR_ERR(data->psy);
>> + dev_err(dev, "failed to register power supply %s\n",
>> + psy_desc->name);
>> + goto err_psy;
>> + }
>> +
>> + dev_info(dev, "sbsm registered\n");
>> + return 0;
>> +
>> +err_psy:
>> +err_mux_register:
>> + i2c_mux_del_adapters(data->muxc);
>> +
>> +err_mux_alloc:
>> + return ret;
>> +}
>> +
>> +static int sbsm_remove(struct i2c_client *client)
>> +{
>> + struct sbsm_data *data = i2c_get_clientdata(client);
>> +
>> + i2c_mux_del_adapters(data->muxc);
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id sbsm_ids[] = {
>> + { "sbs-manager", 0 },
>> + { "ltc1760", 0 },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, sbsm_ids);
>> +
>> +static struct i2c_driver sbsm_driver = {
>> + .driver = {
>> + .name = "sbsm",
>> + .owner = THIS_MODULE,
>
> Please drop .owner assignment. It will be done by i2c-core.
> But you should add the following:
>
> .of_match_table = of_match_ptr(sbsm_dt_ids),
>
>> + },
>> + .probe = sbsm_probe,
>> + .remove = sbsm_remove,
>> + .id_table = sbsm_ids
>> +};
>> +module_i2c_driver(sbsm_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz@schneider-inet.de>");
>> +MODULE_DESCRIPTION("SBSM Smart Battery System Manager");
>
> -- Sebastian
>
--
Regards
Phil Reid
^ permalink raw reply
* Re: [PATCHv2 1/3] dt-bindings: display: Intel FPGA VIP drm driver Devicetree bindings
From: Ong, Hean Loong @ 2017-05-02 2:10 UTC (permalink / raw)
To: robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
dinguyen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Loh, Tien Hock,
Vetter, Daniel, Ong@rob-hp-laptop,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170428183226.7u4sses4gzan6g5e@rob-hp-laptop>
On Fri, 2017-04-28 at 13:32 -0500, Rob Herring wrote:
> On Tue, Apr 25, 2017 at 10:06:44AM +0800, hean.loong.ong@intel.com
> wrote:
> >
> > From: "Ong, Hean Loong" <hean.loong.ong@intel.com>
> >
> > Device tree binding for Intel FPGA Video and Image
> > Processing Suite. The binding involved would be generated
> > from the Altera (Intel) Qsys system. The bindings would
> > set the max width, max height, buts per pixel and memory
> > port width. The device tree binding only supports the Intel
> > Arria10 devkit and its variants. Vendor name retained as
> > altr.
> >
> > Signed-off-by: Ong, Hean Loong <hean.loong.ong@intel.com>
> > ---
> > v2:
> > * Moved Device Tree bindings to
> > Documentation/devicetree/bindings/display/
> > * Added vendor name altr, to description
> > ---
> > .../devicetree/bindings/display/altr,vip-fb2.txt | 30
> > ++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/altr,vip-
> > fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-
> > fb2.txt
> > new file mode 100644
> > index 0000000..bdffefb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> > @@ -0,0 +1,30 @@
> > +Intel Video and Image Processing(VIP) Frame Buffer II bindings
> > +
> > +Supported hardware: Arria 10 and above with display port IP
> > +
> > +The drm driver for the Arria 10 devkit would require the display
> > resolution
> Bindings describe h/w. DRM driver is a Linux term.
>
Noted.
> >
> > +and pixel information to be included as these values are generated
> > based
> > +on the FPGA design that drives the video connector attached to the
> > drm driver
> > +Information the FPGA video IP component can be acquired from
> > +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/li
> > terature/ug/ug_vip.pdf
> > +
> > +Required properties:
> > +
> > +- compatible: "altr,vip-frame-buffer-2.0"
> > +- reg: Physical base address and length of the framebuffer
> > controller's
> > + registers.
> > +- altr,max-width: The width of the framebuffer in pixels.
> > +- altr,max-height: The height of the framebuffer in pixels.
> > +- altr,bits-per-symbol: only "8" is currently supported
> Supported in the driver or IP? The former isn't relevant to the
> binding.
> In the latter case, you don't need it if that's the only thing
> supported.
>
Since the device is an FPGA the values here are based on how the FPGA
HW design is created or programmed. The values here are the optimal
reference design proposed for the Intel Arria10 devkit.
However anyone that uses the Intel Arria10 devkit could create a device
that runs with a different resolution that has varying values but with
the condition that they need to fill these values accordingly with
Intel Quartus Programmer tools.
Once programmed the parameters could not be changed at runtime and the
HW rerence designs currently only support 1 type of resolution per
design.
Therefore the driver needs to support a hardware with varying
parameters programmed specifically into the FPGA.
> >
> > +- altr,mem-port-width = the bus width of the avalon master port on
> > the frame reader
> In bits or bytes?
>
> >
> > +
> > +Example:
> > +
> > + dp_0_frame_buf: vip@100000280 {
> > + compatible = "altr,vip-frame-buffer-2.0";
> > + reg = <0x00000001 0x00000280 0x00000040>;
> > + altr,max-width = <1280>;
> > + altr,max-height = <720>;
> > + altr,bits-per-symbol = <8>;
> > + altr,mem-port-width = <128>;
> > + };
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: Document the STM32 QSPI bindings
From: Brian Norris @ 2017-05-02 1:09 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: Ludovic Barre, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
Alexandre Torgue, devicetree-u79uwXL29TY76Z2rM5mHXA,
Richard Weinberger, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
David Woodhouse
In-Reply-To: <b62272c9-027c-9c9b-42f0-1056d88aac7c-yU5RGvR974pGWvitb5QawA@public.gmane.org>
On Sun, Apr 16, 2017 at 06:53:06PM +0200, Cyrille Pitchen wrote:
> Hi all,
>
> Rob, is this version ok for you? If so, I can take it into the
> github/spi-nor tree.
I see this was missing from your pull request. Rob has since acked,
so...
Applied to l2-mtd.git
--
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
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