Re: [PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT
On Tuesday 05 January 2016 17:26:06 Jon Mason wrote:
> On Fri, Dec 18, 2015 at 10:44:28PM +0100, Arnd Bergmann wrote:
> > On Friday 18 December 2015 16:37:56 Jon Mason wrote:
> > > + cru: cru@1800c184 {
> > > + compatible = "syscon";
> > > + reg = <0x1800c184 0xc>;
> > > + };
> >
> > It's unusual for a device to start at such an odd address. Are you sure
> > it's not a larger device starting at 0x1800c000 or 0x1800?
>
> The CRU (Clock and Reset Unit) starts at 0x1800c100, with the
> following layout:
>
> CRU Clock Management at 0x1800c100-0x1800c180
> CRU Reset at 0x1800c184
> CRU Period Sample Clock at 0x1800c188
> CRU Interrupt register at 0x1800c18c
> CRU MDIO Control at 0x1800c190
> CRU GPIO at 0x1800c1c0-0x1800c1e0
> CRU SDIO 0x1800c200-0x1800c214
> CRU RoboSW Interrupt at 0x1800c280
> CRU Straps Control at 0x1800c2a0
>
> The clock driver is already referencing the registers between
> 0x1800c100-0x1800c180, and the GPIO driver is referencing registers
> between 0x1800c1c0-0x1800c1e0.
>
> The reset part of the syscon seems to be the only useful thing in this
> block. Am I approaching this incorrectly?
I think the problem is how the gpio controller has a device node
that overlaps with one of the devices of the chip.
If the data sheet lists a "Clock and Reset Unit" at those addresses,
that is a strong indication that this is actually a specific
piece of hardware, and it should be represented as one device node
in DT, with the sub-registers being exposed by that driver in
some way. A typical way would be to have a syscon node like
cru: syscon@1800c100 {
compatible = "brcm,bcm53010-clock-reset-unit", "syscon";
reg = <0x1800c100 100>;
};
that represents the entire unit. You can then have syscon references
in each driver that uses it, and/or create a high-level driver that
binds to the "brcm,bcm53010-clock-reset-unit" compatible string and that
exports a set of functions for other drivers to be used if you prefer
to do this as functional abstraction rather than register based.
> > Also, please provide a more specific compatible string based on the
> > name of the device in the data sheet. The node name in contrast should
> > be more generic, e.g.
> >
> > cru: system-controller@1800c000 {
> > compatible = "brcm,bcm53010-cru", "syscon";
>
> This is very similar between the NS and NSP (and NS2) platforms. I'll
> verify the layout and see if this can't be "brcm,iproc-cru" or
> something similar.
If it's only "very similar" but not identical, don't use the same compatible
string, at least not as the only one. You should be able to identify the
specific device by looking at its compatible string in case you want to
write a high-level driver that knows about the differences later (the driver
should not need to inquire the chip name, it should only look at one device
node).
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/dts: Add missing DMA Abort interrupt to Juno
On Thursday 07 January 2016 12:01:59 Robin Murphy wrote: > The DMA-330 has an "irq_abort" interrupt line on which it signals faults > separately from the "irq[n:0]" channel interrupts. On Juno, this is > wired up to SPI 92; add it to the DT so that DMAC faults are correctly > reported for the driver to reset the thing, rather than leaving it > locked up and waiting to time out. > > CC: Liviu Dudau > CC: Sudeep Holla > CC: Lorenzo Pieralisi > Signed-off-by: Robin Murphy > Nothing wrong with the patch, but could you please come up with a more structured way to get patches for Juno into the kernel? You have addressed the patch "to:" the arm-soc maintainers, but you are not listed in the maintainers file for the directory, so it's not clear what you expect to happen here. Ideally, we'd get patches from just one of the people listed in the MAINTAINERS file normally, and let us know if the primary maintainer changes, or if one of the others sends a patch because that person is unavailable. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM64: tegra: Add support for Google Pixel C
On Thursday 07 January 2016 13:19:44 Jon Hunter wrote:
>
> Adding Arnd.
>
> Hmmm ... well apparently stdout-path does not work for tegra and in
> order to make this work I had to do the following ...
>
>
> [PATCH] serial: 8250: of: Add earlycon support for Tegra
>
> Currently, early console support only works for Tegra when the serial
> port information is passed via the earlycon boot parameter. If the
> serial port information is specified via device-tree using the
> "stdout-path" then the early console does not work because:
>
> 1. The tegra serial ports compatibility parameter does not match any
>of the supported serial drivers for early console.
> 2. The of_setup_earlycon() function assumes that serial port registers
>are byte aligned and for tegra they are 32-bit aligned.
>
> Add an early console setup function for tegra so that the early console
> can be specified via the device-tree "stdout-path" variable.
>
> Signed-off-by: Jon Hunter
> ---
> drivers/tty/serial/8250/8250_of.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_of.c
> b/drivers/tty/serial/8250/8250_of.c
> index 33021c1f7d55..98adf83e83c2 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -44,6 +44,16 @@ void tegra_serial_handle_break(struct uart_port *p)
> udelay(1);
> } while (1);
> }
> +
> +int __init tegra_earlycon_setup(struct earlycon_device *device,
> + const char *options)
> +{
> + device->port.iotype = UPIO_MEM32;
> + device->port.regshift = 2;
> +
> + return early_serial8250_setup(device, options);
> +}
> +OF_EARLYCON_DECLARE(tegra20_uart, "nvidia,tegra20-uart",
> tegra_earlycon_setup);
> #else
> static inline void tegra_serial_handle_break(struct uart_port *port)
> {
>
>
> Arnd, does the above look ok, or should there be a generic
> early_serial8250x32_setup() somewhere?
I think it would be better to put it into 8250_early.c rather than
8250_of.c, as there are already some other definitions in there,
and the #ifdef CONFIG_TEGRA in 8250_of.c is for some other workaround.
Would it be possible to handle the "reg-io-width" parsing in
early_serial8250_setup instead of keying it off the string?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] ARM64 LPC: update binding doc
On Tuesday 05 January 2016 19:59:49 Rongrong Zou wrote: > 在 2016/1/5 0:34, Arnd Bergmann 写道: > > On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote: > >> 在 2016/1/4 19:13, Arnd Bergmann 写道: > >>> On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote: > >>>> 在 2015/12/31 23:00, Rongrong Zou 写道: > >> Ranges property can set empty, but this means 1:1 translation. the I/O > >> port range is translated to MMIO address 0x0001 to > >> 0x0001 0004, it looks wrong else. I wonder if anyone get legacy > >> I/O port resource from dts. > > > > As I said, nothing should really require the ranges property here, unless > > you have a valid IORESOURCE_MEM translation. The code that requires > > the ranges to be present is wrong. > > > > I think the openfirmware(DT) do not support for those unmapped I/O ports, > because I > must get resource by calling of_address_to_resource(), which have to call > pci_address_to_pio() when resource type is IORESOURCE_IO. I'm sorry I have no > better idea for this now. Maybe liviu can give me some opinions. I think on x86 it works (or used to work, few people use open firmware on x86 these days, and it may be broken), and the pci_address_to_pio() call behaves differently when PCI_IOBASE is set. x86 never maps I/O ports into memory mapped I/O addresses, they have their own way of accessing them just like your platform. > /** > * of_address_to_resource - Translate device tree address and return as > resource > * > * Note that if your address is a PIO address, the conversion will fail if > * the physical address can't be internally converted to an IO token with > * pci_address_to_pio(), that is because it's either called to early or it > * can't be matched to any host bridge IO space > */ > int of_address_to_resource(struct device_node *dev, int index, > struct resource *r) The problem here seems to be that the code assumes that either the I/O ports are always mapped or they are never mapped (no PCI_IOBASE). We need to extend it because now we can have the combination of the two. > >> For ipmi driver, I can get I/O port resource by DMI rather than dts. > > > > No, the ipmi driver uses the resource that belongs to the platform > > device already, you can't rely on DMI data to be present there. > > Ipmi has a lot of way to be discovered(ACPI, DMI, hardcoded, hot-add, > openfirmware and a few other), I think we just use one of them, not all of > them. > It depend on vendor's hardware solution actually. I don't think we should mix multiple methods here: if the bus is described in DT, all its children should be there as well. Otherwise you get into problems e.g. if you have multiple instances of the LPC bus and the Linux I/O addresses for one or more of them have an offset to the bus specific addresses. The bus probe code decides what the Linux I/O port numbers are, but DMI and other methods have no idea of the mapping. As long as there is only one instance, using the first 0x1000 addresses with a 1:1 mapping saves us a bit of trouble, but I'd be worried about relying on that assumption too much. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of/platform: export of_default_bus_match_table
On Tuesday 05 January 2016 19:50:31 Masahiro Yamada wrote: > 2016-01-05 19:13 GMT+09:00 Arnd Bergmann : > > On Tuesday 05 January 2016 11:17:53 Masahiro Yamada wrote: > > This bus is used to connect external (on-board) devices with the SoC. > > So, no possibility for AMBA, but > I want "simple-bus" for grouping some device nodes. > Also, "simple-mfd" would be useful because an external device could be an MFD. > > > Please see arch/arm/boot/dts/uniphier-support-card.dtsi > > It describes an add-on card (expansion board) > which has ETHER, UART, etc. on it. > > I implemented it as "simple-bus". > > This card is connected to the chip select 1 > of the UniPhier System Bus. > Ok, got it. So you could alternatively define your own match table in the driver with just "simple-bus" and "simple-mfd". I think either way is fine, let's see what the DT maintainers prefer. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] ARM64: dts: mt8173: Add CPU OPP, clock and regulator supply properties
On Monday 28 December 2015 08:43:40 Viresh Kumar wrote: > On 27-12-15, 14:21, Pi-Cheng Chen wrote: > > Add operating-points-v2, clock, and regulator supply properties > > required by mt8173-cpufreq driver to enable it. > > > > Signed-off-by: Pi-Cheng Chen > > --- > > This patch is based on the patch[1] that adds underlying clock MUX for > > MT8173 which is needed by mt8173-cpufreq driver but not yet picked. > > > > [1] http://article.gmane.org/gmane.linux.kernel.clk/325 > > --- > > arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 18 ++ > > arch/arm64/boot/dts/mediatek/mt8173.dtsi| 90 > > + > > 2 files changed, 108 insertions(+) > > Acked-by: Viresh Kumar > This patch is now in linux-next through Rafael's pm tree, and it breaks the arm64 build: arm64-defconfig Error: ../arch/arm64/boot/dts/mediatek/mt8173.dtsi:132.24-25 syntax error Please revert. In the future, please send all dts changes through the proper maintainer channels (-> Mattias -> arm-soc), and make sure they actually build. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of/platform: export of_default_bus_match_table
On Tuesday 05 January 2016 11:17:53 Masahiro Yamada wrote:
> Currently, drivers/bus/uniphier-system-bus.c is kept from being a
> module due to the unresolved reference to of_default_bus_match_table.
>
> Refer to commit 326ea45aa827 ("bus: uniphier: allow only built-in
> driver").
>
> Signed-off-by: Masahiro Yamada
> ---
>
> drivers/of/platform.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index af98343..8d103e4 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -31,6 +31,7 @@ const struct of_device_id of_default_bus_match_table[] = {
> #endif /* CONFIG_ARM_AMBA */
> {} /* Empty terminated list */
> };
> +EXPORT_SYMBOL(of_default_bus_match_table);
I wonder if the uniphier bus should actually use the default
match table at all. Sorry for not having thought of that when
I did my patch.
What kinds of devices do you see below this bus? Do you have multiple
levels of devices? Are they all platform devices or could they
be AMBA?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access
On Monday 04 January 2016 15:20:58 Russell King - ARM Linux wrote:
> On Mon, Jan 04, 2016 at 04:17:47PM +0100, Arnd Bergmann wrote:
> > On Saturday 02 January 2016 14:17:46 Mark Brown wrote:
> > > On Sat, Jan 02, 2016 at 12:19:52AM +0100, Arnd Bergmann wrote:
> > >
> > > > - if (i2c_client->dev.of_node) {
> > > > + if (IS_ENABLED(CONFIG_OF) && i2c_client->dev.of_node) {
> > >
> > > This would be a lot nicer if there was an __always_null annotation we
> > > could put on of_node for !OF configurations, that'd Just Work and this
> > > can't be the only case where we have this idiom.
> > >
> >
> > How about an inline helper like
> >
> > static inline struct device_node *dev_of_node(struct device *dev)
> > {
> > if (IS_ENABLED(CONFIG_OF))
> > return dev->of_node;
>
> ITYM:
>
> return IS_ENABLED(CONFIG_OF) ? dev->of_node : NULL;
>
> or
>
> if (IS_ENABLED(CONFIG_OF))
> return dev->of_node;
> else
> return NULL;
>
Right, yes.
That reminds of a different problem that has been bugging me for a
while: We frequently have a pattern like
#ifdef CONFIG_FOO
static int function(void)
{
...
}
#endif
struct operations = {
...
#ifdef CONFIG_FOO
.function = function;
#endif
...
};
Except that people constantly get it wrong, e.g. by using the
wrong ifdef, forgetting one of the two ifdefs, or by leaving
unused static functions that only get called indirectly from the
other one that is built conditionally.
We could add a macro like
#define COND_PTR(config, ptr) (IS_ENABLED(config) ? (ptr) : NULL)
and then let the compiler figure out that "function" is unused even
without an explicit __maybe_unused annotation. The function above
can be simplied to
static inline struct device_node *dev_of_node(struct device *dev)
{
return COND_PTR(CONFIG_OF, dev->of_node);
}
with that, which is another benefit.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] ARM64 LPC: update binding doc
On Tuesday 05 January 2016 00:04:19 Rongrong Zou wrote:
> 在 2016/1/4 19:13, Arnd Bergmann 写道:
> > On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
> >> 在 2015/12/31 23:00, Rongrong Zou 写道:
> >> */
> >> compatible = "low-pin-count";
> >> device_type = "isa";
> >> #address-cells = <2>;
> >> #size-cells = <1>;
> >> reg = <0x0 0xa01b 0x0 0x1>;
> >> ranges = <0x1 0x0 0x0 0x0 0x1000>;
> >> /*
> >> * ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg
> >> = <0x1, 0x00e4, 4>".
> >> *
> >> */
> >> ipmi_0:ipmi@00e4{
> >> device_type = "ipmi";
> >> compatible = "ipmi-bt";
> >> reg = <0x1 0x00e4 0x4>;
> >> };
> >>
> >
> > This looks wrong: the property above says that the I/O port range is
> > translated to MMIO address 0x to 0x0001, which is not
> > true on your hardware. I think this needs to be changed in the code
> > so the ranges property is not required for I/O ports.
>
> Ranges property can set empty, but this means 1:1 translation. the I/O
> port range is translated to MMIO address 0x0001 to
> 0x0001 0004, it looks wrong else. I wonder if anyone get legacy
> I/O port resource from dts.
As I said, nothing should really require the ranges property here, unless
you have a valid IORESOURCE_MEM translation. The code that requires
the ranges to be present is wrong.
> For ipmi driver, I can get I/O port resource by DMI rather than dts.
No, the ipmi driver uses the resource that belongs to the platform
device already, you can't rely on DMI data to be present there.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access
On Saturday 02 January 2016 14:17:46 Mark Brown wrote:
> On Sat, Jan 02, 2016 at 12:19:52AM +0100, Arnd Bergmann wrote:
>
> > - if (i2c_client->dev.of_node) {
> > + if (IS_ENABLED(CONFIG_OF) && i2c_client->dev.of_node) {
>
> This would be a lot nicer if there was an __always_null annotation we
> could put on of_node for !OF configurations, that'd Just Work and this
> can't be the only case where we have this idiom.
>
How about an inline helper like
static inline struct device_node *dev_of_node(struct device *dev)
{
if (IS_ENABLED(CONFIG_OF))
return dev->of_node;
}
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] ARM64 LPC: update binding doc
On Sunday 03 January 2016 20:24:14 Rongrong Zou wrote:
> 在 2015/12/31 23:00, Rongrong Zou 写道:
> > 2015-12-31 22:40 GMT+08:00 Arnd Bergmann > <mailto:a...@arndb.de>>:
> > > On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
> > > > 在 2015/12/30 17:06, Arnd Bergmann 写道:
> > > > > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> > >
> > > The DT sample above looks good in principle. I believe what you are
> > missing
> > > here is code in your driver to scan the child nodes to create the
> > platform
> > > devices. of_bus_isa_translate() should work with your definition here
> > > and create the correct IORESOURCE_IO resources. You don't have any MMIO
> > > resources, so the absence of a ranges property is ok. Maybe all you
> > > are missing is a call to of_platform_populate() or
> > of_platform_bus_probe()?
> > >
> >
> > You are right. thanks, i'll try on test board . if i get the correct
> > result , the new patch
> > will be sent later. By the way, it's my another email account use when i at
> > home.
>
> I tried, and there need some additional changes.
>
> isa@a01b {
>
> /*the node name should start with "isa", because of below definition
> * static int of_bus_isa_match(struct device_node *np)
> * {
> * return !strcmp(np->name, "isa");
> * }
Looks good. It would be nicer to match on device_type than on name,
but this is ancient code and it's probably best not to touch it
so we don't accidentally break some old SPARC or PPC system.
> */
> compatible = "low-pin-count";
> device_type = "isa";
> #address-cells = <2>;
> #size-cells = <1>;
> reg = <0x0 0xa01b 0x0 0x1>;
> ranges = <0x1 0x0 0x0 0x0 0x1000>;
> /*
> * ranges is required, then i can get the IORESOURCE_IO <0xe4,4> from "reg =
> <0x1, 0x00e4, 4>".
> *
> */
> ipmi_0:ipmi@00e4{
> device_type = "ipmi";
> compatible = "ipmi-bt";
> reg = <0x1 0x00e4 0x4>;
> };
>
This looks wrong: the property above says that the I/O port range is
translated to MMIO address 0x to 0x0001, which is not
true on your hardware. I think this needs to be changed in the code
so the ranges property is not required for I/O ports.
> drivers\of\address.c
> static int __of_address_to_resource(struct device_node *dev,
> const __be32 *addrp, u64 size, unsigned int flags,
> const char *name, struct resource *r)
> {
> u64 taddr;
>
> if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
> return -EINVAL;
> taddr = of_translate_address(dev, addrp);
> if (taddr == OF_BAD_ADDR)
> return -EINVAL;
> memset(r, 0, sizeof(struct resource));
> if (flags & IORESOURCE_IO) {
> unsigned long port;
>
> /*/
> /*legacy port(< 0x1000) is reserved, and need no translation here*/
> /*/
> if(taddr + size < PCIBIOS_MIN_IO){
> r->start = taddr;
> r->end = taddr + size - 1;
> }
I don't like having a special case based on the address here,
the same kind of hack might be needed for PCI I/O spaces in
hardware that uses an indirect method like your LPC bus
does, and the code above will not work on any LPC implementation
that correctly multiplexes its I/O ports with the first PCI domain.
I think it would be better to avoid translating the port into
a physical address to start with just to translate it back into
a port number, what we need instead is the offset between the
bus specific port number and the linux port number. I've added
Liviu to Cc, he wrote this code originally and may have some idea
of how we could do that.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mmc: omap_hsmmc: Add support for slot-name property in DT
On Sunday 03 January 2016 00:03:54 Pali Rohár wrote:
> On Saturday 02 January 2016 23:57:47 Arnd Bergmann wrote:
> > On Saturday 02 January 2016 16:22:03 Pali Rohár wrote:
> > > On Monday 28 December 2015 15:55:28 Arnd Bergmann wrote:
> > > > On Monday 28 December 2015 15:54:35 Pali Rohár wrote:
> > > > > > I mean you can add the platform data to the
> > > > > > omap_auxdata_lookup[] table for this board.
> > > > >
> > > > > But can I mix data from omap3-n900.dts and
> > > > > omap_auxdata_lookup[]?
> > >
> > > Hm... looks like it is not possible. omap_hsmmc driver ignores any
> > > supplied platform data if there are device tree data. So array
> > > omap_auxdata_lookup[] does not help.
> >
> > Obviously you need to the driver to work with that setting. Maybe
> > something as simple as
> >
> > diff --git a/drivers/mmc/host/omap_hsmmc.c
> > b/drivers/mmc/host/omap_hsmmc.c index e06b1881b6a1..4fa35fc84b8b
> > 100644
> > --- a/drivers/mmc/host/omap_hsmmc.c
> > +++ b/drivers/mmc/host/omap_hsmmc.c
> > @@ -2006,7 +2006,7 @@ static int omap_hsmmc_probe(struct
> > platform_device *pdev) void __iomem *base;
> >
> > match = of_match_device(of_match_ptr(omap_mmc_of_match),
> > &pdev->dev); -if (match) {
> > + if (!pdata && match) {
> > pdata = of_get_hsmmc_pdata(&pdev->dev);
> >
> > if (IS_ERR(pdata))
> >
>
> But in this case I must copy mmc definition from omap3-n900.dts file
> back to board code to omap_auxdata_lookup[]. And mmc definitions in
> omap3-n900.dts will be ignored. This is step back.
>
> Mixing mmc definitions from DTS file together with omap_auxdata_lookup[]
> just will not work.
As I said earlier, if you prefer to avoid the auxdata hack, you are
also welcome to work on support for named slots in the MMC core code,
it will just be more work and will take time to get consensus on.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mmc: omap_hsmmc: Add support for slot-name property in DT
On Saturday 02 January 2016 16:22:03 Pali Rohár wrote:
> On Monday 28 December 2015 15:55:28 Arnd Bergmann wrote:
> > On Monday 28 December 2015 15:54:35 Pali Rohár wrote:
> > > >
> > > > I mean you can add the platform data to the omap_auxdata_lookup[]
> > > > table for this board.
> > >
> > > But can I mix data from omap3-n900.dts and omap_auxdata_lookup[]?
> >
> Hm... looks like it is not possible. omap_hsmmc driver ignores any
> supplied platform data if there are device tree data. So array
> omap_auxdata_lookup[] does not help.
Obviously you need to the driver to work with that setting. Maybe
something as simple as
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index e06b1881b6a1..4fa35fc84b8b 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -2006,7 +2006,7 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
void __iomem *base;
match = of_match_device(of_match_ptr(omap_mmc_of_match), &pdev->dev);
- if (match) {
+ if (!pdata && match) {
pdata = of_get_hsmmc_pdata(&pdev->dev);
if (IS_ERR(pdata))
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gpiolib: always initialize *flags from of_get_named_gpio_flags
The of_get_named_gpio_flags() function does nothing other than returning
an error when CONFIG_OF_GPIO is disabled, but that causes spurious
warnings about possible use of uninitialized variables in any code that
does not check the of_get_named_gpio_flags() return value before trying
to use the flags:
drivers/input/misc/rotary_encoder.c: In function 'rotary_encoder_probe':
drivers/input/misc/rotary_encoder.c:223:28: warning: 'flags' may be used
uninitialized in this function [-Wmaybe-uninitialized]
drivers/power/bq24735-charger.c: In function 'bq24735_charger_probe':
drivers/power/bq24735-charger.c:227:12: warning: 'flags' may be used
uninitialized in this function [-Wmaybe-uninitialized]
drivers/power/sbs-battery.c: In function 'sbs_probe':
drivers/power/sbs-battery.c:782:17: warning: 'gpio_flags' may be used
uninitialized in this function [-Wmaybe-uninitialized]
This changes the behavior of the inline helper to set the flags to zero
when OF_GPIO is disabled, to avoid the warnings. In all cases I've
encountered, we don't actually get to the place that uses the flags
if CONFIG_OF is disabled because we won't enter the DT parser code.
Signed-off-by: Arnd Bergmann
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index 87d6d1632dd4..bb85a8eeba6a 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -67,6 +67,9 @@ extern int of_gpio_simple_xlate(struct gpio_chip *gc,
static inline int of_get_named_gpio_flags(struct device_node *np,
const char *list_name, int index, enum of_gpio_flags *flags)
{
+ if (flags)
+ *flags = 0;
+
return -ENOSYS;
}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 RESEND 0/2] WM8505/WM8650 DT fixes for SD card controller
On Friday 01 January 2016 20:32:30 Roman Volkov wrote: > > Applied both to next/dt, thanks a lot for following up! > > > > Let me know if you think this should go into stable backports as well, > > I did not apply it to the fixes branch as you don't have a > > 'Cc: sta...@vger.kernel.org' tag and it has never worked so far. > > Yes, this must go into the stable too. Let me know if I must change > something or resend. I can put them in the fixes branch with the appropriate stable tag myself, but please clarify whether we need just the first or both patches there. It looks to me that the second one while correct only addresses a cosmetic problem and everything works without it. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 RESEND 0/2] WM8505/WM8650 DT fixes for SD card controller
On Friday 01 January 2016 16:38:10 Roman Volkov wrote: > From: Roman Volkov > > This patch set enables SD controller support for WM8650 and > fixes minor errors in WM8505 Device Tree file. > > Changes in v3: > 1. Add minor fixes for WM8505 SDHC node > > Tested on both WM8505 and WM8650. > > Roman Volkov (2): > dts: vt8500: Add SDHC node to DTS file for WM8650 > dts: vt8500: Fix errors in SDHC node for WM8505 > > arch/arm/boot/dts/wm8505.dtsi | 4 ++-- > arch/arm/boot/dts/wm8650.dtsi | 9 + > 2 files changed, 11 insertions(+), 2 deletions(-) > > -- > Hi maintainers, I see my previous versions were not applied. Could this > little patch set be applied for the linux-next? I don't think this is new > functionality, this must be considered as bugfix for existing Device Tree. > > Any other suggestions? > Applied both to next/dt, thanks a lot for following up! Let me know if you think this should go into stable backports as well, I did not apply it to the fixes branch as you don't have a 'Cc: sta...@vger.kernel.org' tag and it has never worked so far. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: uniphier: fix a typo in comment block
On Thursday 24 December 2015 00:23:40 Masahiro Yamada wrote:
>
> -/* UART3 unavilable: the pads are not wired to the package balls */
> +/* UART3 unavailable: the pads are not wired to the package balls */
> &serial3 {
> status = "disabled";
> };
>
Applied on next/dt, thanks!
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] ARM64: Enable SP805 WDT support for FSL LS2080A
On Monday 28 December 2015 15:31:22 Bhupesh Sharma wrote: > Hi Arnd, Kevin, Olof > > This is the v4 of patchset which adds the support for SP805 WDT on FSL LS2080A > and also adds the missing documentation of SP805 WDT device-tree bindings. > > Rebased against arm-soc/next/dt > Applied all on next/dt64, thanks! Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] WM8505/WM8650 DT fixes for SD card controller
On Thursday 31 December 2015 16:31:02 Arnd Bergmann wrote: > On Thursday 24 December 2015 00:48:25 Roman Volkov wrote: > > В Sat, 4 Apr 2015 15:27:20 +0300 > > Roman Volkov пишет: > > > > > В Sun, 1 Mar 2015 23:39:11 +0300 > > > Roman Volkov пишет: > > > > > > > В Sun, 01 Mar 2015 20:52:55 +0100 > > > > Arnd Bergmann пишет: > > > > > > Any other suggestions? > > > > > > > > > > According to the MAINTAINERS file, Tony Prisk should be the person > > > > > to pick them up, but he was not the recipient of the mail. > > > > > > > > > > Arnd > > > > > > > > Thanks for the tip. Tony probably need to add these files to his > > > > list of maintained files. get_maintainer.pl doesn't mention him. > > > > > > > > Roman > > > > > > Arnd, > > > > > > No response yet from Tony for over a month. > > > > > > Roman > > > > Ping. At least two patch sets for vt8500 are waiting to be picked up. > > vt8500 maintainer is inactive for over a year. > > I seem to have lost the original mails now, as it's been a while. Can you > send the entire series of patches to a...@kernel.org please? To clarify, please first make sure they still apply cleanly on a recent kernel, then send them to a...@kernel.org with all the involved parties and the mailing list(s) in Cc. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] WM8505/WM8650 DT fixes for SD card controller
On Thursday 24 December 2015 00:48:25 Roman Volkov wrote: > В Sat, 4 Apr 2015 15:27:20 +0300 > Roman Volkov пишет: > > > В Sun, 1 Mar 2015 23:39:11 +0300 > > Roman Volkov пишет: > > > > > В Sun, 01 Mar 2015 20:52:55 +0100 > > > Arnd Bergmann пишет: > > > > > Any other suggestions? > > > > > > > > According to the MAINTAINERS file, Tony Prisk should be the person > > > > to pick them up, but he was not the recipient of the mail. > > > > > > > > Arnd > > > > > > Thanks for the tip. Tony probably need to add these files to his > > > list of maintained files. get_maintainer.pl doesn't mention him. > > > > > > Roman > > > > Arnd, > > > > No response yet from Tony for over a month. > > > > Roman > > Ping. At least two patch sets for vt8500 are waiting to be picked up. > vt8500 maintainer is inactive for over a year. I seem to have lost the original mails now, as it's been a while. Can you send the entire series of patches to a...@kernel.org please? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier
On Thursday 31 December 2015 17:16:34 James Liao wrote: > > """ > Take a example for our IOMMU(M4U) and SMI. The IOMMU which is > subsys_init defaultly will depend on SMI. > The SMI is a bridge between m4u and the Multimedia HW. About the HW > block diagram and more other information please help check [1]. > SMI is responsible to enable/disable iommu and help transfer data for > each Multimedia HW, Both have to wait until the power and the clocks is > enabled. > > So our iommu should probe done after smi, smi should be after > power-domain, and all the iommu consumer(display/vdec/venc/camera etc.) > should be after the iommu. > Then all the multimedia module will be delayed by power-domain who is > module_init currently. > > After grep, we get some example whose pm is not module_init: > core_initcall(exynos4_pm_init_power_domain); > subsys_initcall(imx_pgc_init); > > So we expect move the power-domain initial more earlier too. The > power-domain seems to be a basic module like ccf. > Is there some special reason about we should use module_init, or do you > have any concern if we change it? > Thanks. Ok, got it. Generally, we should try to avoid using the earlier initcall levels, but a few things like clock controllers, iommus etc are special enough that we need to make sure their dependencies are there by the time those are probed. Please put your explanation above into the patch changelog and add a code comment about the IOMMU next to the subsys_initcall() so it doesn't accidentally get changed when someone tries to do a code cleanup. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] ARM64 LPC: update binding doc
On Thursday 31 December 2015 22:12:19 Rongrong Zou wrote:
> 在 2015/12/30 17:06, Arnd Bergmann 写道:
> > On Tuesday 29 December 2015 21:33:52 Rongrong Zou wrote:
> >> +Example:
> >> +
> >> +lpc_0: lpc@a01b {
> >> + #address-cells = <1>;
> >> + #size-cells = <1>;
> >> +compatible = "low-pin-count";
> >> +reg = <0x0 0xa01b 0x0 0x1>;
> >> +};
> >
> > One more thought: please try to stick as closely as possible to the existing
> > ISA binding that is documented at
> >
> > http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> From the specification, I think I should use 2 32bit integer to describe the
> isa addr in dts.
> >
> > In particular, this should cover the possibility of describing both memory
> > and I/O spaces in child devices.
> >
>
> I found below config in powerpc dts "arch/powerpc/boot/dts/mpc8544ds.dts"
>
> isa@1e {
> device_type = "isa";
> #interrupt-cells = <2>;
> #size-cells = <1>;
> #address-cells = <2>;
> reg = <0xf000 0x0 0x0 0x0 0x0>;
> ranges = <0x1 0x0 0x100 0x0 0x0
>0x1000>;
> interrupt-parent = <&i8259>;
>
>
>
> rtc@70 {
> compatible = "pnpPNP,b00";
> reg = <0x1 0x70 0x2>;
> };
> the isa space in child-node: reg = <0x1 0x70 0x2>;
> 0x1 means IO space, 70 means addr, 0x2 is size.
> but when i config the following in dts, the ipmi_0 node can't be probed,
> I think there may be some problems.
>
> lpc_0: lpc@a01b {
> compatible = "low-pin-count";
> device_type = "isa";
> #address-cells = <2>;
> #size-cells = <1>;
> reg = <0x0 0xa01b 0x0 0x1>;
>
> ipmi_0:ipmi@00e4{
> device_type = "ipmi";
> compatible = "ipmi-bt";
> reg = <0x1 0x00e4 0x4>;
> };
The DT sample above looks good in principle. I believe what you are missing
here is code in your driver to scan the child nodes to create the platform
devices. of_bus_isa_translate() should work with your definition here
and create the correct IORESOURCE_IO resources. You don't have any MMIO
resources, so the absence of a ranges property is ok. Maybe all you
are missing is a call to of_platform_populate() or of_platform_bus_probe()?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier
On Wednesday 30 December 2015 18:12:08 James Liao wrote: > On Wed, 2015-12-30 at 09:52 +0100, Arnd Bergmann wrote: > > On Wednesday 30 December 2015 14:41:44 James Liao wrote: > > > Some power domain comsumers may init before module_init. > > > So the power domain provider (scpsys) need to be initialized > > > earlier too. > > > > > > Signed-off-by: James Liao > > > --- > > > > > > > Why? > > Some drivers use different init level to ensure they can be initialized > before other drivers. To support these drivers, moving scpsys driver's > initial function to subsys_init is the most easy way. This is just the same generic explanation that you already have. Please be more specific what the dependency is and why we can't rely on deferred probing here. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] clk: bcm2835: Add bindings for the auxiliary peripheral clock gates.
On Tuesday 29 December 2015 16:15:09 Rob Herring wrote: > On Mon, Dec 28, 2015 at 4:39 PM, Michael Turquette > wrote: > > Quoting Eric Anholt (2015-12-24 15:45:15) > >> Michael Turquette writes: > >> I would *love* to do that, but I've previously been told that having the > >> bindings patch reference a header file not present as of the bindings > >> patch is not acceptable and made to change it. > > > > Ugh, that is annoying. I would think that having code compile properly > > would trump the desire to have all of the documentation merged as one > > patch. > > What about compiling the dts? > > > On the other hand, I've been asked to not take binding descriptions > > through the clk tree. That is a policy that I'm happy to comply with, > > but it is at odds with the recommendation for the header and the binding > > description to be merged together. > > By who? Any bindings in a series I always expect the subsystem > maintainers to take the whole series. That doesn't solve the problem > though as there is still a dependency between a subsystem tree and > arm-soc typically. I don't care too much which tree the binding description goes through either, as long as it is kept in sync. > > DT folks, what is the right way to do this? An immutable, shared branch > > just for a single header file solves the problem, but also feels very > > cumbersome for such a trivial issue. > > Arnd and Olof have been complaining about this problem which is worse > when it is a binding, driver and dts. > > I'm open to maintaining a branch for this purpose if that helps. That > or staggering merging of bindings and drivers/dts are the only ideas > I've come up with. > > How about allowing binding descriptions to be merged without the header > > file, so long as it is merged through another tree? > > I think that is wrong if we have the goal to separate bindings from > the kernel and the bindings should stand on their own. However, if it > greatly simplifies things, i'd be okay with that. The header file is really the main issue we need to worry about. My preferred way of doing this would be to give it an extra merge window: add the binding document and the header file in one merge window, and then add the dts files and the driver one release later. I've seen a lot of header files added for no good reason at all, and at least that way we can get people to think about the dependency more. It's also ok to merge the header file and binding with either the dts file changes or the driver and then do the other part the following release. In the past, we've worked around the issue by merging the driver through arm-soc, or by merging the dts changes through a driver tree, with the appropriate Acks in each case. Both of those approaches work of course, but the former always feels awkward to me as we are not using the right maintainer path, and the latter approach tends to cause merge conflicts, especially when multiple headers for different subsystems get added or the dts files are added at the same time. Having a shared branch for the header file is another way to do it, and we can do that in some cases, but I'd prefer not to make it the default. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] soc: mediatek: Init MT8173 scpsys driver earlier
On Wednesday 30 December 2015 14:41:44 James Liao wrote: > Some power domain comsumers may init before module_init. > So the power domain provider (scpsys) need to be initialized > earlier too. > > Signed-off-by: James Liao > --- > Why? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] soc: mediatek: Separate scpsys driver common code
On Wednesday 30 December 2015 14:41:43 James Liao wrote: > diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig > index 0a4ea80..eca6fb7 100644 > --- a/drivers/soc/mediatek/Kconfig > +++ b/drivers/soc/mediatek/Kconfig > @@ -22,11 +22,20 @@ config MTK_PMIC_WRAP > > config MTK_SCPSYS > bool "MediaTek SCPSYS Support" > - depends on ARCH_MEDIATEK || COMPILE_TEST > - default ARM64 && ARCH_MEDIATEK > select REGMAP > select MTK_INFRACFG > select PM_GENERIC_DOMAINS if PM > help > Say yes here to add support for the MediaTek SCPSYS power domain > driver. > + > +config MTK_SCPSYS_MT8173 > + bool "MediaTek MT8173 SCPSYS Support" > + depends on ARCH_MEDIATEK || COMPILE_TEST > + select MTK_SCPSYS > + default ARCH_MEDIATEK > Please don't "select" a user-visible Kconfig symbol. Either hide MTK_SCPSYS by removing the string behind 'bool', or make this "depends on MTK_SCPSYS". Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 0/6] misc: add reboot mode driver
On Monday 28 December 2015 16:56:12 Heiko Stübner wrote:
> Am Montag, 28. Dezember 2015, 16:28:55 schrieb Arnd Bergmann:
> > On Wednesday 23 December 2015 17:31:45 Andy Yan wrote:
> > > +{ .compatible = "rockchip,reboot-mode-nvram",
> > > +.data = (void *)&reboot-mode-nvram },
> > > +{},
> > > +};
> >
> > nvram is a complex topic by itself, because there are so many ways to do it.
> > I think what you are referring to here is a battery-backed memory that uses
> > one or more bytes at a fixed offset to store a particular piece of
> > information, as the drivers/char/nvram.c driver does. Maybe we should put
> > the reboot mode into that driver then?
> >
> > There are other nvram drivers at various places in the kernel, and each may
> > be slightly different, or completely different, like the EFIVARs driver on
> > UEFI firmware or the key/value store on Open Firmware, these probably need
> > their own methods and not share the generic driver.
>
> actually we now have drivers/nvmem/* that does that byte-wise addressing for
> eeproms, efuses, etc in a generic way.
Good point, so some of the reboot-reason users will be able to use that
framework, but some will not:
- drivers/nvmem only works for fixed-length data in a fixed location, but
not for key/value pairs, TLVs etc.
- Coming back to an earlier problem I pointed out, a lot of the things
stored in an nvram need a consistent user interface. This includes stuff
like kernel command line, boot image name, boot device, console
configuration, network mac address. We probably want the reboot reason
to fit into the wider framework for these, and drivers/nvmem doesn't
(currently) look like a good place for that.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver
On Monday 28 December 2015 10:20:56 Thierry Reding wrote: > > > > HTC apparently uses a separate RAM area to pass the reboot reason, > > > > and they have a driver to store that, which is separate from the > > > > driver that they use for actually rebooting the machine. > > > > > > I wasn't very clear, but the PMC_SCRATCH0 register is used to store the > > > reset reason. It supports the recovery mode, which I think is really an > > > Android thing, "bootloader" will typically cause the bootloader not to > > > boot anything, and "forced-recovery" will go into a recovery mode that > > > is used to bootstrap the device (usually by uploading a "miniloader" > > > that initializes RAM, downloads a bootloader for booting or flashing an > > > operating system, ...). > > > > > > The write that resets the SoC is to a different register. > > > > So is this scratch register interpreted by some maskrom code, or by code > > that > > can be provided by the OEM? > > My understanding is that its interpreted both by what's called BootROM > on Tegra (I guess that's what you call "maskrom code") and the system's > bootloader. The BootROM cannot typically be replaced by the OEM, but it > is quite typical for the bootloader to differ between devices. Ok, so not maskrom (which would not be OEM specific, but hardcoded for the chip) but rather some form of PROM. This means we can only guess that all OEMs use the same protocol but in theory someone could have implemented an incompatible BootROM, but it's also possible that HTC just ignore the register entirely and implement the same thing separately. > The BootROM will interpret a subset of the bits in that register. Most > notable the "force recovery" bit. That will cause the BootROM to go into > a mode which can be used to bootstrap the device. It implements a simple > protocol (RCM) which can be used to upload a loader (usually referred to > as mini-loader) to the device's IRAM which in turn will initialize the > memory and which can download a proper bootloader (such as U-Boot) to > memory using a slightly more complex protocol (the standard protocol is > called nv3p, but the particular choice of protocol no longer matters at > this point). > > The bootloader can also access this register and will interpret the > "bootloader" and "recovery" bits. On the, argueably small, sample of > Android devices I've worked with, the "bootloader" bit will make the > bootloader go into fastboot mode, whereas the "recovery" bit will make > it initiate the RCK mode, which is used to update through OTA. > > There are a couple of other bits in this register, but they are badly > documented and don't seem to relate to reboot. Ok. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 0/6] misc: add reboot mode driver
On Wednesday 23 December 2015 17:31:45 Andy Yan wrote:
>
> I also have the idea to put is in drivers/power/reset, But considering
> this driver have not bind anything about power, so I put it in
> driver/misc
> at last. So I hope if some people can give more suggestions here.
I vote for drivers/power/reset as well. On some platforms, the two things
are related, on others they are not, but it's good to have it all in one
place I think.
> >> drivers/soc/rockchip/Kconfig | 9 ++
> >> drivers/soc/rockchip/Makefile | 1 +
> >> drivers/soc/rockchip/reboot.c | 68 +++
> > And maybe that part could be made generic instead of rockchip specific.
> > It simply uses a regmap to do the accesses, I guess a lot of other
> > platforms will do that. We have syscon-reboot and syscon-poweroff for
> > example.
> >
> > I think then we can extend the "framework" by having generic drivers to
> > store the value in eeprom or nvram for example.
> >
>
> I also hope the write interface can be generic. But I found some
> platform
> use different hardware to store the value. For example, John's patch
> use
> SRAM on qcom apq8064 to store value for nexus7. It seems there also have
> some platform use dram or nvram to store it. And these different
> hardware use
> different write method. I don't have a generic way to handle this.
>
> I have a idea to handle it like this:
>
> +static const struct of_device_id reboot_mode_dt_match[] = {
> +{ .compatible = "linux,reboot-mode-sfr",/*for magic value
> stored in special function register, which can be accessed by regmap*/
> +.data = (void *)&reboot-mode-sfr },
I'd make this one syscon-reboot-mode, to go along with syscon-poweroff
as implemented by drivers/power/reset/syscon-poweroff.c. The syscon concept
is already generic enough that we don't need a linux prefix for that.
> +{ .compatible = "linux,reboot-mode-sram", /*for magic value
> stored in
> +.data = (void *)&reboot-mode-sram },
the sram mode should probably follow the generic SRAM binding and make
the location that stores the reboot mode a separate partition, unless
we want to store other data in the same partition, in which case we might
want to use the same implementation as for the nvram.
> +{ .compatible = "linux,reboot-mode-sdram",
> +.data = (void *)&reboot-mode-sdram }, /*for magic value
> stored
I think "sdram" is not an appropriate name here, as the main system memory
might use some other technology, e.g. DRAM or DDR2-SDRAM.
> +{ .compatible = "rockchip,reboot-mode-nvram",
> +.data = (void *)&reboot-mode-nvram },
> +{},
> +};
nvram is a complex topic by itself, because there are so many ways to do it.
I think what you are referring to here is a battery-backed memory that uses
one or more bytes at a fixed offset to store a particular piece of information,
as the drivers/char/nvram.c driver does. Maybe we should put the reboot mode
into that driver then?
There are other nvram drivers at various places in the kernel, and each may
be slightly different, or completely different, like the EFIVARs driver on
UEFI firmware or the key/value store on Open Firmware, these probably need
their own methods and not share the generic driver.
> the data point to different hardware access method.
>
>Hope to see more suggestions from you.
It's probably best to leave these four examples as separate drivers and we can
add further ones when needed.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mmc: omap_hsmmc: Add support for slot-name property in DT
On Monday 28 December 2015 15:54:35 Pali Rohár wrote: > On Monday 28 December 2015 15:41:01 Arnd Bergmann wrote: > > On Monday 28 December 2015 15:28:48 Pali Rohár wrote: > > > On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote: > > > > On Friday 25 December 2015 13:53:11 Pali Rohár wrote: > > > > > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote: > > > > > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote: > > > > > > > * Arnd Bergmann [150515 14:26]: > > > > > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote: > > > > > > > If setting up the generic binding is expected to take a > > > > > > > while, you can naturally pass it in pdata while waiting > > > > > > > for the generic binding to get merged. > > > > > > > > > > > > Yes, good idea. So the n900 machine can use auxdata to pass > > > > > > this for the time being, while the binding and generic > > > > > > implementation is being worked out. > > > > > > > > > > Ok, so what is needed to finish this patch series? > > > > > > > > I don't know where we are at this point. Has either the auxdata > > > > approach or the generic binding been worked on at all? > > > > > > What are auxdata data? > > > > I mean you can add the platform data to the omap_auxdata_lookup[] > > table for this board. > > But can I mix data from omap3-n900.dts and omap_auxdata_lookup[]? Yes. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mmc: omap_hsmmc: Add support for slot-name property in DT
On Monday 28 December 2015 15:28:48 Pali Rohár wrote: > On Monday 28 December 2015 15:14:50 Arnd Bergmann wrote: > > On Friday 25 December 2015 13:53:11 Pali Rohár wrote: > > > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote: > > > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote: > > > > > * Arnd Bergmann [150515 14:26]: > > > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote: > > > > > If setting up the generic binding is expected to take a while, > > > > > you can naturally pass it in pdata while waiting for the > > > > > generic binding to get merged. > > > > > > > > Yes, good idea. So the n900 machine can use auxdata to pass this > > > > for the time being, while the binding and generic implementation > > > > is being worked out. > > > > > > Ok, so what is needed to finish this patch series? > > > > I don't know where we are at this point. Has either the auxdata > > approach or the generic binding been worked on at all? > > What are auxdata data? I mean you can add the platform data to the omap_auxdata_lookup[] table for this board. > And what do you mean with "generic binding" approach? Start a discussion to specify slot names and implement that in the mmc driver core, so we can remove the hack from the OMAP driver and make it work the same way for any machine. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mmc: omap_hsmmc: Add support for slot-name property in DT
On Friday 25 December 2015 13:53:11 Pali Rohár wrote: > On Monday 18 May 2015 17:07:57 Arnd Bergmann wrote: > > On Monday 18 May 2015 08:06:07 Tony Lindgren wrote: > > > * Arnd Bergmann [150515 14:26]: > > > > On Friday 15 May 2015 23:22:37 Pali Rohár wrote: > > > If setting up the generic binding is expected to take a while, > > > you can naturally pass it in pdata while waiting for the generic > > > binding to get merged. > > > > Yes, good idea. So the n900 machine can use auxdata to pass this for > > the time being, while the binding and generic implementation is > > being worked out. > > > > Ok, so what is needed to finish this patch series? I don't know where we are at this point. Has either the auxdata approach or the generic binding been worked on at all? If not, please try to get the auxdata variant to work, it should not be hard at all because there is no dependency on a generic binding. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] dts/ls2080a: Update DTSI to add support of SP805 WDT
On Wednesday 23 December 2015, Bhupesh SHARMA wrote: > > Hi Arnd, Olof, Kevin > > On Tue, Dec 15, 2015 at 8:00 PM, Bhupesh Sharma > wrote: > > This patch updates the LS2080a DTSI (DTS Include) file to add > > support for eight SP805 Watchdog units which can be used to > > reset the eight Cortex-A57 cores available on LS2080A. > > > > Signed-off-by: Bhupesh Sharma > > --- > > arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 56 > > > > 1 file changed, 56 insertions(+) > > Since Rob has acked the SP805 WDT binding documentation patch (patch > 1/2 of this series), > can this DTSI change, please be considered for absorption in the arm-soc tree. Sure, please submit it for inclusion. Who is the maintainer for arch/arm64/boot/dts/freescale/ ? Can they pick up all the freescale related dts changes and submit them as a pull request? Should they go through Shawn's tree maybe? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] Raspberry Pi 2 support.
On Tuesday 22 December 2015, Alexander Aring wrote: > On Tue, Dec 22, 2015 at 12:11:32AM +0100, Arnd Bergmann wrote: > > On Sunday 20 December 2015, Alexander Aring wrote: > > > On Fri, Dec 18, 2015 at 11:08:26AM +0100, Alexander Aring wrote: > > > > [0.00] Memory policy: Data cache writeback > > > > [0.00] BUG: mapping for 0x3f201000 at 0xf0201000 out of vmalloc > > > > space > > > > This is a separate bug, right? > > > > Yes, I can remove this BUG when setting: > > VMSPLIT_2G > > or some other option, was "VMSPLIT_3G" before and with this option I > will get the above message. Please try to figure out why this happened, it should really work with any vmsplit, in particular the one that everyone uses. > > > > [0.00] [ cut here ] > > > > [0.00] WARNING: CPU: 0 PID: 0 at arch/arm/kernel/devtree.c:149 > > > > arm_dt_init_cpu_maps+0x100/0x1a4() > > > > [0.00] DT /cpu 2 nodes greater than max cores 1, capping them > > > > [0.00] Modules linked in: > > > > [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc5+ #989 > > > > [0.00] Hardware name: BCM2835 > > > > [0.00] [] (unwind_backtrace) from [] > > > > (show_stack+0x20/0x24) > > > > [0.00] [] (show_stack) from [] > > > > (dump_stack+0x20/0x28) > > > > [0.00] [] (dump_stack) from [] > > > > (warn_slowpath_common+0x9c/0xc4) > > > > [0.00] [] (warn_slowpath_common) from [] > > > > (warn_slowpath_fmt+0x40/0x48) > > > > [0.00] [] (warn_slowpath_fmt) from [] > > > > (arm_dt_init_cpu_maps+0x100/0x1a4) > > > > [0.00] [] (arm_dt_init_cpu_maps) from [] > > > > (setup_arch+0x6f4/0x89c) > > > > [0.00] [] (setup_arch) from [] > > > > (start_kernel+0x74/0x3a4) > > > > [0.00] [] (start_kernel) from [<8078>] (0x8078) > > > > [0.00] ---[ end trace cb88537fdc8fa200 ]--- > > > > [0.00] DT missing boot CPU MPIDR[23:0], fall back to default > > > > cpu_logical_map > > > > [0.00] CPU: All CPU(s) started in SVC mode. > > > > > > I can remove this WARNING when I enable CONFIG_SMP. > > > > I think we should try to change this in the code. > > > > mhhh, okay... I am not sure I think there is some missing > IS_ENABLED(CONFIG_SMP) check. Makes sense. > > > > [0.00] Virtual kernel memory layout: > > > > [0.00] vector : 0x - 0x1000 ( 4 kB) > > > > [0.00] fixmap : 0xffc0 - 0xfff0 (3072 kB) > > > > [0.00] vmalloc : 0xf080 - 0xff80 ( 240 MB) > > > > [0.00] lowmem : 0xc000 - 0xf000 ( 768 MB) > > > > [0.00] modules : 0xbf00 - 0xc000 ( 16 MB) > > > > [0.00] .text : 0xc0008000 - 0xc074a7cc (7434 kB) > > > > [0.00] .init : 0xc074b000 - 0xc07b4000 ( 420 kB) > > > > [0.00] .data : 0xc07b4000 - 0xc081e910 ( 427 kB) > > > > [0.00].bss : 0xc081e910 - 0xc08ca80c ( 688 kB) > > > > [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, > > > > Nodes=1 > > > > [0.00] NR_IRQS:16 nr_irqs:16 16 > > > > [0.00] Kernel panic - not syncing: /soc/local_intc: unable to > > > > map local interrupt registers > > > > [0.00] > > > > [0.00] CPU: 0 PID: 0 Comm: swapper Tainted: GW > > > > 4.4.0-rc5+ #989 > > > > [0.00] Hardware name: BCM2835 > > > > [0.00] [] (unwind_backtrace) from [] > > > > (show_stack+0x20/0x24) > > > > [0.00] [] (show_stack) from [] > > > > (dump_stack+0x20/0x28) > > > > [0.00] [] (dump_stack) from [] > > > > (panic+0x84/0x210) > > > > [0.00] [] (panic) from [] > > > > (bcm2836_arm_irqchip_l1_intc_of_init+0x94/0x110) > > > > [0.00] [] (bcm2836_arm_irqchip_l1_intc_of_init) from > > > > [] (of_irq_init+0x1a0/0x2a8) > > > > [0.00] [] (of_irq_init) from [] > > > > (irqchip_init+0x14/0x1c) > > > > [0.00] [] (irqchip_init) from [] > > > > (init_IRQ+0x28/0x88) > > > > [0.00] [] (init_IRQ) from [] > > >
Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.
On Tuesday 22 December 2015, David Daney wrote: > On 12/22/2015 02:07 AM, Will Deacon wrote: > > On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote: > >> From: David Daney > >> diff --git a/drivers/pci/host/pci-host-generic.c > >> b/drivers/pci/host/pci-host-generic.c > >> index 5434c90..e83cec7 100644 > >> --- a/drivers/pci/host/pci-host-generic.c > >> +++ b/drivers/pci/host/pci-host-generic.c > [...] > >> -static int gen_pci_probe(struct platform_device *pdev) > >> +int gen_pci_common_probe(struct platform_device *pdev, > >> + struct gen_pci *pci) > > > > Whilst I'm fine with this patch, I don't know how Bjorn will feel about > > exposing this function outside of the generic host driver. We could avoid > > it by turning things upside-down and having the generic driver probe > > the other drivers by matching a compatible string with a probe function > > pointer, but I'd be interested to see what others think. > > > > Note: I know that pci-host-generic is not built as a loadable module, but... > > struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the > registering of platform drivers is fairly well standardized in the > kernel, and module loading userpace tools. Agreed, this is the correct way to do the abstraction if we want one, the way that Will describes is generally not a good idea, and we've converted a number of drivers that did it like that to the way you do it here. > The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the > same module as the driver for the device. We are creating a separate > driver precisely because we don't want to mix all this ThunderX specific > code into the pci-host-generic driver when it is used by arm-32bit and > others. This means that, at a minimum, we would have to export the > pci-host-generic probe function so that it could be referenced by struct > platform_driver in other modules. Right. > This brings up the next problem. How to attach driver specific data to > the generic driver structures? At first I tried augmenting struct > gen_pci_cfg_bus_ops with a callback .init() function to be called by the > generic driver, but this would also require adding an an element to > struct gen_pci to point to a driver specific data object. It felt a > little convoluted and complex. > > This led me to the current design where struct gen_pci is embedded in > the driver specific structure, and the allocation of this is done in the > driver specific probe function. No more callbacks, no additions to the > pci-host-generic structures. I think it is a little cleaner this way. > > If there are suggestions as to how it can be made cleaner yet, I would > be happy to implement and test them. My idea of the long-term direction for the pci-host-generic driver would be to move more parts into the PCI core code as library functions that can be used by other drivers as well. This would also address my other concern that I'd like to see the generic host driver remain the simplest example that we have, and only require any additional code in other drivers to add functionality or workarounds. Adding an abstraction layer within the driver to some degree goes in the opposite direction of that. One approach that might work would be to split the existing driver into three files: one for CAM, one for ECAM and one for the common parts, with an interface similar to what you have here. Then you can add your driver as a third front-end, and we can keep working on integrating the common parts further into the PCI core. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] Raspberry Pi 2 support.
On Sunday 20 December 2015, Alexander Aring wrote: > On Fri, Dec 18, 2015 at 11:08:26AM +0100, Alexander Aring wrote: > > [0.00] Memory policy: Data cache writeback > > [0.00] BUG: mapping for 0x3f201000 at 0xf0201000 out of vmalloc > > space This is a separate bug, right? > > [0.00] [ cut here ] > > [0.00] WARNING: CPU: 0 PID: 0 at arch/arm/kernel/devtree.c:149 > > arm_dt_init_cpu_maps+0x100/0x1a4() > > [0.00] DT /cpu 2 nodes greater than max cores 1, capping them > > [0.00] Modules linked in: > > [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc5+ #989 > > [0.00] Hardware name: BCM2835 > > [0.00] [] (unwind_backtrace) from [] > > (show_stack+0x20/0x24) > > [0.00] [] (show_stack) from [] > > (dump_stack+0x20/0x28) > > [0.00] [] (dump_stack) from [] > > (warn_slowpath_common+0x9c/0xc4) > > [0.00] [] (warn_slowpath_common) from [] > > (warn_slowpath_fmt+0x40/0x48) > > [0.00] [] (warn_slowpath_fmt) from [] > > (arm_dt_init_cpu_maps+0x100/0x1a4) > > [0.00] [] (arm_dt_init_cpu_maps) from [] > > (setup_arch+0x6f4/0x89c) > > [0.00] [] (setup_arch) from [] > > (start_kernel+0x74/0x3a4) > > [0.00] [] (start_kernel) from [<8078>] (0x8078) > > [0.00] ---[ end trace cb88537fdc8fa200 ]--- > > [0.00] DT missing boot CPU MPIDR[23:0], fall back to default > > cpu_logical_map > > [0.00] CPU: All CPU(s) started in SVC mode. > > I can remove this WARNING when I enable CONFIG_SMP. I think we should try to change this in the code. > > [0.00] Virtual kernel memory layout: > > [0.00] vector : 0x - 0x1000 ( 4 kB) > > [0.00] fixmap : 0xffc0 - 0xfff0 (3072 kB) > > [0.00] vmalloc : 0xf080 - 0xff80 ( 240 MB) > > [0.00] lowmem : 0xc000 - 0xf000 ( 768 MB) > > [0.00] modules : 0xbf00 - 0xc000 ( 16 MB) > > [0.00] .text : 0xc0008000 - 0xc074a7cc (7434 kB) > > [0.00] .init : 0xc074b000 - 0xc07b4000 ( 420 kB) > > [0.00] .data : 0xc07b4000 - 0xc081e910 ( 427 kB) > > [0.00].bss : 0xc081e910 - 0xc08ca80c ( 688 kB) > > [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1 > > [0.00] NR_IRQS:16 nr_irqs:16 16 > > [0.00] Kernel panic - not syncing: /soc/local_intc: unable to map > > local interrupt registers > > [0.00] > > [0.00] CPU: 0 PID: 0 Comm: swapper Tainted: GW > > 4.4.0-rc5+ #989 > > [0.00] Hardware name: BCM2835 > > [0.00] [] (unwind_backtrace) from [] > > (show_stack+0x20/0x24) > > [0.00] [] (show_stack) from [] > > (dump_stack+0x20/0x28) > > [0.00] [] (dump_stack) from [] > > (panic+0x84/0x210) > > [0.00] [] (panic) from [] > > (bcm2836_arm_irqchip_l1_intc_of_init+0x94/0x110) > > [0.00] [] (bcm2836_arm_irqchip_l1_intc_of_init) from > > [] (of_irq_init+0x1a0/0x2a8) > > [0.00] [] (of_irq_init) from [] > > (irqchip_init+0x14/0x1c) > > [0.00] [] (irqchip_init) from [] > > (init_IRQ+0x28/0x88) > > [0.00] [] (init_IRQ) from [] > > (start_kernel+0x20c/0x3a4) > > [0.00] [] (start_kernel) from [<8078>] (0x8078) > > [0.00] ---[ end Kernel panic - not syncing: /soc/local_intc: unable > > to map local interrupt registers > > > > But still getting this panic, your patches does not contain some > defconfig, so I still try to figure out what I need to enable to get it > working. Maybe it is some missing config entry which should be enabled. > > But, for me it looks like some devicetree issue, because of_iomem returns > NULL, at [0]. Don't know what I am doing wrong. That is the most likely cause, yes. You can try replacing it with an ioremap with a hardcoded physical address to see if that works. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
On Monday 21 December 2015, Bjorn Andersson wrote: > I disagree with this "recommendation" as it's only outcome will be asymmetry. > > I think this script should be changed to only warn if there's a single > IS_ERR/PTR_ERR at the end of the function, not if there's a list of > them and this would replace the last one. > Agreed, looks like a false positive, and I think it's best to ignore the warning here, but not change the script that seems reasonable in general. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3] cpufreq: qoriq: Register cooling device based on device tree
On Tuesday 15 December 2015 00:58:26 Rafael J. Wysocki wrote: > On Thursday, November 26, 2015 05:21:11 PM Jia Hongtao wrote: > > Register the qoriq cpufreq driver as a cooling device, based on the > > thermal device tree framework. When temperature crosses the passive trip > > point cpufreq is used to throttle CPUs. > > > > Signed-off-by: Jia Hongtao > > Reviewed-by: Viresh Kumar > > Applied, thanks! > I got a randconfig build error today: drivers/built-in.o: In function `qoriq_cpufreq_ready': debugfs.c:(.text+0x1f4688): undefined reference to `of_cpufreq_cooling_register' CONFIG_OF=y CONFIG_QORIQ_CPUFREQ=y CONFIG_THERMAL=m CONFIG_THERMAL_OF=y I think you need a 'depends on THERMAL' to prevent the driver from being built-in when THERMAL=m. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT
On Friday 18 December 2015 16:37:56 Jon Mason wrote:
> + cru: cru@1800c184 {
> + compatible = "syscon";
> + reg = <0x1800c184 0xc>;
> + };
It's unusual for a device to start at such an odd address. Are you sure
it's not a larger device starting at 0x1800c000 or 0x1800?
Also, please provide a more specific compatible string based on the
name of the device in the data sheet. The node name in contrast should
be more generic, e.g.
cru: system-controller@1800c000 {
compatible = "brcm,bcm53010-cru", "syscon";
reg = <0x1800c000 0x400>; /* whatever the data sheet says */
};
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I2C eeprom compatibles? (was Re: [PATCH/RFC 03/19] ARM: shmobile: gose: add i2c2 bus to device tree)
On Friday 18 December 2015 08:35:32 Wolfram Sang wrote: > > > > It seems to me that we have some consensus around: > > > > compatible = "renesas,r1ex24002", "24c02"; > > Thinking again, "generic,24c02" or "generic-24c02" could also be an > option. > > > Should this be added to Documentation/devicetree/bindings/eeprom/eeprom.txt > > ? > > Or documented elsewhere? > > Probably we need a DT maintainers advice here? I don't mind vendor > specific compatibles being documented, but I'm reluctant to add all > these compatibles for the myriads of I2C eeproms to the at24 driver. 99% > are covered by the generic case. > > Adding DT to CC. I'd rather use some vendor string in addition to 24c02. Isn't this originally an Atmel part? In that case, using "atmel,24c02" as the most generic string would be appropriate, and IIRC the i2c framework will just match that with the "24c02" entry in the i2c_device_id list. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Raspberry Pi power domains
On Thursday 17 December 2015 11:03:47 Eric Anholt wrote: > >> > >> .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 47 > >> arch/arm/boot/dts/bcm2835-rpi.dtsi | 11 + > >> arch/arm/boot/dts/bcm2835.dtsi | 2 +- > >> arch/arm/mach-bcm/Kconfig | 10 + > >> arch/arm/mach-bcm/Makefile | 1 + > >> arch/arm/mach-bcm/raspberrypi-power.c | 247 > >> + > >> include/dt-bindings/arm/raspberrypi-power.h| 41 > >> include/soc/bcm2835/raspberrypi-firmware.h | 2 + > >> 8 files changed, 360 insertions(+), 1 deletion(-) > >> create mode 100644 > >> Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > >> create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c > >> create mode 100644 include/dt-bindings/arm/raspberrypi-power.h > >> > >> -- > >> 2.6.2 > >> > > > > Besides a nitpick for patch2, I would also reverse the order of patch3 > > and patch2. DT docs should go in before the actual parsing of the new > > bindings/compatibles. > > > > Anyway, for the hole series, you may add my: > > > > Reviewed-by: Ulf Hansson > > Would your tree be pulling the series (since it's power domains), or > should I (since it's SOC stuff)? > All of the above files go through the arm-soc tree by default. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] Raspberry Pi 2 support.
On Thursday 17 December 2015 09:48:57 Stefan Wahren wrote:
> Am 17.12.2015 um 01:37 schrieb Eric Anholt:
> > Arnd Bergmann writes:
> >
> >> On Wednesday 16 December 2015 15:55:07 Eric Anholt wrote:
> >>> This is a major rewrite of the previous Raspberry Pi 2 submission.
> >>> SMP support is now included, and the DT includes are cleaned up to
> >>> avoid massive duplication.
> >>>
> >>> The branch (based on 4.4-rc5, to get the USB regression fixes) can be
> >>> found at:
> >>>
> >>> https://github.com/anholt/linux/tree/bcm2836-4.4
> >> Looks all good to me, but when we get the pull request, I'd strongly
> >> prefer to have that based on -rc3 or earlier.
> >>
> >> What commit is the USB regression fix you refer to above? Is that in a
> >> branch that is -rc3 based? Maybe you can rebase the changes on top
> >> of that branch, to minimize the amount of backmerges?
> > Top 4 commits of drivers/usb/dwc2 for 4.4-rc5 (possibly not all of them
> > are required, but it's what I've been using).
> >
>
> Unfortunately all 4 of them are required:
>
> d0464bcf12af ("usb: dwc2: Make PHY optional")
>
> 6c2dad69163f ("usb: dwc2: Return errors from PHY")
>
> 8aa90cf2a286 ("usb: dwc2: make otg clk optional")
>
> f74875dc3613 ("usb: dwc2: fix kernel oops during driver probe")
Ok, that's good. They are all based on -rc1, so you you can just rebase
on top of f74875dc3613, which was pulled by GregKH.
> But they should apply to -rc3 too.
Please don't rebase them, use the commits that got merged, otherwise we'd
get duplicate commits and that would be worse than the backmerge.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] Raspberry Pi 2 support.
On Wednesday 16 December 2015 15:55:07 Eric Anholt wrote: > This is a major rewrite of the previous Raspberry Pi 2 submission. > SMP support is now included, and the DT includes are cleaned up to > avoid massive duplication. > > The branch (based on 4.4-rc5, to get the USB regression fixes) can be > found at: > > https://github.com/anholt/linux/tree/bcm2836-4.4 Looks all good to me, but when we get the pull request, I'd strongly prefer to have that based on -rc3 or earlier. What commit is the USB regression fix you refer to above? Is that in a branch that is -rc3 based? Maybe you can rebase the changes on top of that branch, to minimize the amount of backmerges? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver
On Wednesday 16 December 2015 16:59:39 Rob Herring wrote: > On Mon, Dec 14, 2015 at 3:35 AM, Arnd Bergmann wrote: > > On Monday 14 December 2015 15:26:11 Peter Chen wrote: > > I agree on doing it properly, but am not sure that pwrseq binding for > MMC is proper. The pwrseq binding is fairly limited and working around > the driver model IMO. Hubs may also need I2C setup which I don't think > could be done generically other than some defined sequence of i2c > transactions. The current project I'm working on needs to use I2C to > configure the hub to use HSIC mode for example. I really think we need > a pre-probe driver hook to handle this. That would allow device > specific setup to live in the driver. > > Perhaps a more simple approach would be just forcing driver probe if a > DT node is present. I'm not all that familiar with USB drivers, but > presumably there is some setup before probe like setting the USB > device address. We'd have to allow doing that later during probe. Yes, good idea. I was also advocating that approach for MMC at some point, but the power sequencing made it in the end. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] net/macb: add support for resetting PHY using GPIO
On Wednesday 16 December 2015 19:31:30 Gregory CLEMENT wrote: > diff --git a/drivers/net/ethernet/cadence/macb.c > b/drivers/net/ethernet/cadence/macb.c > index 88c1e1a..35661aa 100644 > --- a/drivers/net/ethernet/cadence/macb.c > +++ b/drivers/net/ethernet/cadence/macb.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > #include > #include Is this the patch that is already in linux-next? I needed an additional #include to avoid this build error on randconfig builds without GPIOLIB: drivers/net/ethernet/cadence/macb.c: In function 'macb_probe': drivers/net/ethernet/cadence/macb.c:2908:19: error: implicit declaration of function 'devm_gpiod_get_optional' [-Werror=implicit-function-declaration] bp->reset_gpio = devm_gpiod_get_optional(&bp->pdev->dev, "phy-reset", ^ drivers/net/ethernet/cadence/macb.c:2909:8: error: 'GPIOD_OUT_HIGH' undeclared (first use in this function) GPIOD_OUT_HIGH); ^ drivers/net/ethernet/cadence/macb.c:2909:8: note: each undeclared identifier is reported only once for each function it appears in drivers/net/ethernet/cadence/macb.c: In function 'macb_remove': drivers/net/ethernet/cadence/macb.c:2979:3: error: implicit declaration of function 'gpiod_set_value' [-Werror=implicit-function-declaration] Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] serial: support for 16550A serial ports on LP-8x4x
On Wednesday 16 December 2015 11:04:57 Sergei Ianovich wrote: > On Tue, 2015-12-15 at 22:51 +0100, Arnd Bergmann wrote: > > On Wednesday 16 December 2015 00:04:45 Sergei Ianovich wrote: > > > index 000..5f9a4c1 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/serial/lp8x4x-serial.txt > > > @@ -0,0 +1,35 @@ > > > +UART ports on ICP DAS LP-8x4x > > > + > > > +ICP DAS LP-8x4x contains three additional serial ports interfaced > > > via > > > +Analog Devices ADM213EA chips in addition to 3 serial ports on PXA > > > CPU. > > > + > > > +Required properties: > > > +- compatible : should be "icpdas,uart-lp8x4x" > > > > Compatible strings should not include a 'x' wildcard like this, better > > use > > the specific chip name. > > > > Also, it sounds like you named them after the board vendor, which > > sounds > > wrong as the vendor part of the compatible string should be the > > whoever > > made that part (analog?) > > The chips themselves are standard, they would work with 8250_core if > properly connected. However, they are not connected normally. Al least > some of their config pins are wired to a different address region. So > the driver is board-specific. Ok, I see. > 'x' wildcards in the name of the board seem important. There are devices > made by the same vendor without 8 or 4 in their name. Those devices > either are not shipped with linux or are base on a x86 platform. > > Does this justify the choice of the compatible string? What I meant was that you should use the specific numbers of one machine, precisely for the reason you list above. If there is e.g. a LP-8040 and a LP-8141 today, and you use lp8x4x in the compatible string to cover both, this will no longer work when the vendor comes out with a LP8047 that is completely different. Instead, what you should do is to use the compatible string to identify one particular board (e.g. the first one that used this setup), and then list the other ones as compatible with this. You can also add the other board names in addition, e.g. compatible = "icpdas,uart-lp8041", "icpdas,uart-lp8040"; for a lp8041 that is compatible with the lp8040. If it turns out later that they are not entirely compatible, we can work around this in the driver by checking for the lp8041 string that will be matched first, while the lp8040 can be used by the driver to match the entire family of compatible machines (no need to list every one in the driver). Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller
On Wednesday 16 December 2015 02:26:26 Yoshihiro Shimoda wrote:
> Hi Arnd again,
>
> > From: Yoshihiro Shimoda
> > Sent: Wednesday, December 16, 2015 10:43 AM
> < snip >
> > > > +static void usb3_write(struct renesas_usb3 *usb3, u32 data, u32 offs)
> > > > +{
> > > > + iowrite32(data, usb3->reg + offs);
> > > > +}
> > > > +
> > > > +static u32 usb3_read(struct renesas_usb3 *usb3, u32 offs)
> > > > +{
> > > > + return ioread32(usb3->reg + offs);
> > > > +}
> > >
> > > I think using readl() is more common than ioread32() if the driver cannot
> > > use IORESOURCE_IO but only IORESOURCE_MEM.
> > >
> > > On ARM, the two are the same, but on some architectures ioread32 is more
> > > expensive, so using the former is preferred.
> >
> > I will use {read,write}l() instead of io{read,write}32().
> > Also I will change io{read,write}32_rep() functions too.
>
> Oops. If I used {read,write}sl() instead of io{read,write}32_rep(),
> build error happened in x86 environment.
>
> CC [M] drivers/usb/gadget/udc/renesas_usb3.o
> ./drivers/usb/gadget/udc/renesas_usb3.c: In function 'usb3_write_pipe':
> ./drivers/usb/gadget/udc/renesas_usb3.c:879:3: error: implicit declaration of
> function 'writesl' [-Werror=implicit-function-declaration]
>writesl(usb3->reg + fifo_reg, buf, len / 4);
>^
> ./drivers/usb/gadget/udc/renesas_usb3.c: In function 'usb3_read_pipe':
> ./drivers/usb/gadget/udc/renesas_usb3.c:923:3: error: implicit declaration of
> function 'readsl' [-Werror=implicit-function-declaration]
>readsl(usb3->reg + fifo_reg, buf, len / 4);
>
> So, I will keep to use io{read,write}32(_rep) functions.
Sounds ok. I actually thought we had added readsl to all architectures, but
apparently
we only changed some architectures that needed it in the past.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: emac: emac gigabit ethernet controller driver
On Tuesday 15 December 2015 15:09:23 Timur Tabi wrote: > Arnd Bergmann wrote: > > If that's in the probe() called from it function, just use writel() > > everywhere, > > a few extra microseconds won't kill the boot time. In general, if a user > > would > > notice the difference, use the relaxed version and add a comment to explain > > how you proved it's correct, otherwise stay with the default accessors. > > What about adding a wmb() after the last writel()? This driver does > that a lot. Is that something we want to discourage? I can understand > how we would want to make sure that the last write is posted before the > function exits. Please explain in a comment specifically which race you are closing by ensuring that the write gets posted. What does it race against? As I said earlier, guaranteeing that a write gets posted does not mean it has arrived at the device, we only get that behavior after a subsequent read from the same device, but you don't need a wmb() between the write and the read to guarantee this. If you have an odd bus that does not follow those rules, it may in fact be best to have a separate set of I/O accessors and not use readl/writel at all. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] serial: support for 16550A serial ports on LP-8x4x
On Wednesday 16 December 2015 00:04:45 Sergei Ianovich wrote:
> index 000..5f9a4c1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/lp8x4x-serial.txt
> @@ -0,0 +1,35 @@
> +UART ports on ICP DAS LP-8x4x
> +
> +ICP DAS LP-8x4x contains three additional serial ports interfaced via
> +Analog Devices ADM213EA chips in addition to 3 serial ports on PXA CPU.
> +
> +Required properties:
> +- compatible : should be "icpdas,uart-lp8x4x"
Compatible strings should not include a 'x' wildcard like this, better use
the specific chip name.
Also, it sounds like you named them after the board vendor, which sounds
wrong as the vendor part of the compatible string should be the whoever
made that part (analog?)
> +- reg : should provide 16 byte man IO memory region and 1 byte region for
> + termios
> +
> +- interrupts : should provide interrupt
> +
> +- interrupt-parent : should provide a link to interrupt controller either
> +explicitly or implicitly from a parent node
interrupt-parent should be an optional property, or you can leave it out,
as this is a standard property that can always be there when there is
interrupts.
> +Examples (from pxa27x-lp8x4x.dts):
> +
> + uart@9050 {
By convention, the name should be 'serial', not 'uart'.
Arnd.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] arm: pxa27x: support for ICP DAS LP-8x4x w/ DT
On Tuesday 15 December 2015 21:01:35 Robert Jarzmik wrote: > Arnd Bergmann writes: > > > On Tuesday 15 December 2015 21:50:13 Sergei Ianovich wrote: > >> On Tue, 2015-12-15 at 19:06 +0100, Robert Jarzmik wrote: > >> Updated plan: > >> 1. MACH_PXA27X_DT > >> 2. PXA_FB and 8250_PXA to enable console > What about gpio-pxa ? And maybe pinctrl-pxa27x (only in linux-next by now) ? > Can > a pxafb work without gpio-pxa ? Also usually for pxafb you have pwm and pwm_bl > to see something. > > >> 3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC > >> 3.1. MTD, MTD_CFI, MTD_PHYSMAP_OF and JFFS2_FS to enable boot from MTD > Ok. Is pxa2xx-pcmcia out of scope ? I seem to remember we have several boards > where pcmcia is the rootfs (even if that is a small mess right now I still > have > to work on). Or do we consider pcmcia as obsolete ? I'm definitely fine with tossing in everything that is PXA specific, even if it's rarely used. If someone wants a smaller kernel, they can still start out with the defconfig and disable stuff they don't need, which tends to be easier than the opposite, and it gives us compile-time coverage with the autobuilders that compile every defconfig. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver
On Tuesday 15 December 2015 18:42:36 Thierry Reding wrote: > On Tue, Dec 15, 2015 at 05:34:00PM +0100, Arnd Bergmann wrote: > > On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote: > > > On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote: > > > > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote: > > > > > rockchip platform have a protocol to pass the kernel reboot > > > > > mode to bootloader by some special registers when system reboot. > > > > > By this way the bootloader can take different action according > > > > > to the different kernel reboot mode, for example, command > > > > > "reboot loader" will reboot the board to rockusb mode, this is > > > > > a very convenient way to get the board enter download mode. > > > > > > > > > > Signed-off-by: Andy Yan > > > > > > > > Adding John Stultz to Cc > > > > > > > > I just saw this thread pop up again, and had to think of John's recent > > > > patch to unify this across platforms. > > > > > > > > John, can you have a look at this driver too, and see how it fits in? > > > > I think this is yet another variant, using an MMIO register rather than > > > > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise > > > > it conceptually fits in with what you had. > > > > > > FWIW, Tegra typically does use an MMIO register as well. See > > > drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what > > > HTC does, but if it's writing somewhere in RAM it isn't using the > > > standard way of resetting the SoC. There's early boot ROM code which I > > > think evaluates the PMC_SCRATCH0 register on Tegra to determine which > > > mode to boot into. That's before even any firmware gets the chance of > > > doing anything. I just checked the android lollipop tree, and I could not find a pmc_restart_notify function, only this file https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/drivers/htc_debug/stability/reboot_params.c with the device that stores into RAM. It looks like HTC ported over a driver that they were already using on some Qualcomm MSM8960 device, as in https://gitlab.com/MaC/android_kernel_htc_msm8960/blob/859977fc723f59a6b707df1d70e80826ee1dccc4/arch/arm/mach-msm/htc/htc_restart_handler.c On Android marshmallow (Flounder), that file again does not exist, and I don't see how it's done. > > HTC apparently uses a separate RAM area to pass the reboot reason, > > and they have a driver to store that, which is separate from the > > driver that they use for actually rebooting the machine. > > I wasn't very clear, but the PMC_SCRATCH0 register is used to store the > reset reason. It supports the recovery mode, which I think is really an > Android thing, "bootloader" will typically cause the bootloader not to > boot anything, and "forced-recovery" will go into a recovery mode that > is used to bootstrap the device (usually by uploading a "miniloader" > that initializes RAM, downloads a bootloader for booting or flashing an > operating system, ...). > > The write that resets the SoC is to a different register. So is this scratch register interpreted by some maskrom code, or by code that can be provided by the OEM? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] arm: pxa27x: support for ICP DAS LP-8x4x w/ DT
On Tuesday 15 December 2015 21:50:13 Sergei Ianovich wrote: > On Tue, 2015-12-15 at 19:06 +0100, Robert Jarzmik wrote: > > > 4. all optional pxa stuff as modules > > > 5. all stuff on supported pxa boards as modules > > > > > If #5 is extended when support for new boards is added, it should be > > > possible to run any supported pxa27x board with pxa27x_defconfig. > > > > I'm very eager to see a patch on that. I can feed my Jenkins with it, > > it would > > greatly help me catch issues earlier. Moreover it would for free test > > it on > > lubbock, mainstone and mioa701. If there was one also for pxa3xx, I > > would launch > > it on zylonite cm-x300 ... (that's a bonus, I know :)) > > > > Updated plan: > 1. MACH_PXA27X_DT > 2. PXA_FB and 8250_PXA to enable console > 3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC > 3.1. MTD, MTD_CFI, MTD_PHYSMAP_OF and JFFS2_FS to enable boot from MTD > 4. all optional pxa stuff as modules > 5. all stuff on supported pxa boards as modules > 6. supported boards should boot the kernel built with > `pxa27x-dt_defconfig` after `make olddefconfig` > > It is probably a good idea to put this plan somewhere in Documentation/ > and to have a comment about that in the defconfig itself. > > Is the plan acceptable? Sounds good to me. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] arm: pxa27x: support for ICP DAS LP-8x4x w/ DT
On Tuesday 15 December 2015 19:42:07 Sergei Ianovich wrote: > On Tue, 2015-12-15 at 17:32 +0100, Arnd Bergmann wrote: > > On Tuesday 15 December 2015 19:27:50 Sergei Ianovich wrote: > > > .../devicetree/bindings/vendor-prefixes.txt| 1 + > > > arch/arm/boot/dts/Makefile | 3 + > > > arch/arm/boot/dts/pxa27x-lp8x4x-i105.dts | 50 > > > arch/arm/boot/dts/pxa27x-lp8x4x.dts| 259 > > > + > > > arch/arm/configs/lp8x4x_defconfig | 176 > > > ++ > > > > > > > I had not noticed earlier that you are adding a new defconfig file. > > PXA is > > already the platform with the most defconfig files, and I'd rather > > like > > to see that reduced than increased. > > > > Is there a chance you could merge this one with some of the existing > > files > > into one configuration that handles them all? > > There are several board-specific devices on LP8x4x: custom FPGA, custom > UART, custom IRQ on FPGA, custom parallel bus for industrial IO. The > defconfig file could alert potential users to this fact. If this is not > a sufficient reason to have a defconfig file, it can be dropped. > > I use the full .config anyway I would like those drivers to be enabled in some defconfig, so we get compile-time coverage, but we generally stopped having one-config-per-board files. Maybe we can have a pxa_defconfig file that enables lots of boards and then we remove the individual configs? We don't have to remove them all at once, but it would make me very happy if we could at least kill off some of the ones that are not used regularly. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver
On Tuesday 15 December 2015 17:31:22 Thierry Reding wrote: > On Mon, Dec 14, 2015 at 12:39:44PM +0100, Arnd Bergmann wrote: > > On Wednesday 18 November 2015 17:56:22 Andy Yan wrote: > > > rockchip platform have a protocol to pass the kernel reboot > > > mode to bootloader by some special registers when system reboot. > > > By this way the bootloader can take different action according > > > to the different kernel reboot mode, for example, command > > > "reboot loader" will reboot the board to rockusb mode, this is > > > a very convenient way to get the board enter download mode. > > > > > > Signed-off-by: Andy Yan > > > > Adding John Stultz to Cc > > > > I just saw this thread pop up again, and had to think of John's recent > > patch to unify this across platforms. > > > > John, can you have a look at this driver too, and see how it fits in? > > I think this is yet another variant, using an MMIO register rather than > > RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise > > it conceptually fits in with what you had. > > FWIW, Tegra typically does use an MMIO register as well. See > drivers/soc/tegra/pmc.c:tegra_pmc_restart_notify(). I don't know what > HTC does, but if it's writing somewhere in RAM it isn't using the > standard way of resetting the SoC. There's early boot ROM code which I > think evaluates the PMC_SCRATCH0 register on Tegra to determine which > mode to boot into. That's before even any firmware gets the chance of > doing anything. HTC apparently uses a separate RAM area to pass the reboot reason, and they have a driver to store that, which is separate from the driver that they use for actually rebooting the machine. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] arm: pxa27x: support for ICP DAS LP-8x4x w/ DT
On Tuesday 15 December 2015 19:27:50 Sergei Ianovich wrote: > .../devicetree/bindings/vendor-prefixes.txt| 1 + > arch/arm/boot/dts/Makefile | 3 + > arch/arm/boot/dts/pxa27x-lp8x4x-i105.dts | 50 > arch/arm/boot/dts/pxa27x-lp8x4x.dts| 259 > + > arch/arm/configs/lp8x4x_defconfig | 176 ++ > I had not noticed earlier that you are adding a new defconfig file. PXA is already the platform with the most defconfig files, and I'd rather like to see that reduced than increased. Is there a chance you could merge this one with some of the existing files into one configuration that handles them all? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: emac: emac gigabit ethernet controller driver
On Tuesday 15 December 2015 09:17:00 Timur Tabi wrote: > Arnd Bergmann wrote: > > We generally want to use readl/writel rather than the relaxed versions, > > unless it is in performance-critical code. > > What about if we have 20+ writes in a row, for example, when > initializing a part? I've seen code like this: > > writel_relaxed(...); > writel_relaxed(...); > writel_relaxed(...); > ... > writel(...); // HW now inited, so enable If that's in the probe() called from it function, just use writel() everywhere, a few extra microseconds won't kill the boot time. In general, if a user would notice the difference, use the relaxed version and add a comment to explain how you proved it's correct, otherwise stay with the default accessors. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: emac: emac gigabit ethernet controller driver
On Tuesday 15 December 2015 09:30:16 Christopher Covington wrote:
>
> On 12/14/2015 08:39 PM, Florian Fainelli wrote:
> > On 14/12/15 16:19, Gilad Avidov wrote:
>
> >> +static void emac_mac_irq_enable(struct emac_adapter *adpt)
> >> +{
> >> +int i;
> >> +
> >> +for (i = 0; i < EMAC_NUM_CORE_IRQ; i++) {
> >> +struct emac_irq *irq = &adpt->irq[i];
> >> +const struct emac_irq_config*irq_cfg =
> >> &emac_irq_cfg_tbl[i];
> >> +
> >> +writel_relaxed(~DIS_INT, adpt->base + irq_cfg->status_reg);
> >> +writel_relaxed(irq->mask, adpt->base + irq_cfg->mask_reg);
> >> +}
> >> +
> >> +wmb(); /* ensure that irq and ptp setting are flushed to HW */
> >
> > Would not using writel() make the appropriate thing here instead of
> > using _relaxed which has no barrier?
>
> It appears to me that the barrier in writel() comes before the access
> [1]. The barrier in this code comes after the accesses. In addition to
> the ordering, if you're suggesting all writel_relaxed be switched out,
> that would seem to add 7 unnecessary barriers, which could adversely
> affect performance.
>
> 1. http://lxr.free-electrons.com/source/arch/arm64/include/asm/io.h#L130
You are right, the writel does not flush the write out to hardware,
and generally that is not needed, in particular since most buses do
not actually wait for a write to complete when a barrier is issued.
I'm missing two explanations here:
a) How performance-critical is the emac_mac_irq_enable() function?
Is this only called when configuring the device, or each time
you call napi_complete()?
b) What other code relies on the write being flushed out first?
Can you move the barrier to the other side? If emac_mac_irq_enable()
is called a lot, you might be able to avoid that barrier altogether
if you instead put it whereever you access the device that requires
the interrupts to be enabled.
> >> +mta = readl_relaxed(adpt->base + EMAC_HASH_TAB_REG0 + (reg << 2));
> >> +mta |= (0x1 << bit);
> >> +writel_relaxed(mta, adpt->base + EMAC_HASH_TAB_REG0 + (reg << 2));
> >> +wmb(); /* ensure that the mac address is flushed to HW */
> >
> > This is getting too much here, just use the correct I/O accessor for
> > your platform, period.
>
> Based on your previous comment, I'm guessing you're suggesting using
> readl() and writel() here instead of *_relaxed and an explicit wmb().
> Again it's not clear to me why swapping the barrier-access ordering and
> adding an additional barrier would result in more correct code.
We generally want to use readl/writel rather than the relaxed versions,
unless it is in performance-critical code.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: berlin4ct: switch to Cortex-A53 specific pmu nodes
On Tuesday 15 December 2015 22:36:12 Jisheng Zhang wrote: > - compatible = "arm,armv8-pmuv3"; > + compatible = "arm,cortex-a53-pmu"; > Should this list both? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] phy: use syscon framework APIs to set ctrl mod reg
On Tuesday 15 December 2015 16:44:41 Kishon Vijay Abraham I wrote: > Hi Arnd, > > On Tuesday 15 December 2015 04:26 PM, Arnd Bergmann wrote: > > On Tuesday 15 December 2015 14:45:59 Kishon Vijay Abraham I wrote: > >> This series is basically to deprecate using phy-omap-control and use > >> syscon APIs to program the control module registers. > >> > >> Changes from v2: > >> No changes. > >> > >> Changes from v1: > >> *) cleanup ti_pipe3_probe in multiple steps > >> *) other minor cleanups > >> > >> Changes from [1] in PHY patches include > >> *) cleanup ti_pipe3_probe > >> *) have mask, power_on and power_off values in usb_phy_data for > >>omap-usb2 phy > >> > >> The patches have been pushed to > >> git://git.ti.com/linux-phy/linux-phy.git syscon > >> > >> [1] -> https://lkml.org/lkml/2015/6/23/189 > >> > >> All the testing was done both before applying the dt patches and after > >> applying the dt patches (dt patches will be posted shortly). > >> > > > > Can you explain here what the conversion is good for? Why do you > > prefer the syscon mapping over a high-level driver in this case? > > phy-omap-control driver was added when there was no proper > infrastructure for doing control module initializations. The > phy-omap-control driver is not an 'actual' PHY driver and it > was just a hack to do PHY related control module initializations. > phy-omap-control is also getting unmanageable with the number of > platforms each having number of modules (like USB, SATA, PCIe), > using the same driver for control module initializations. > > Now with SYSCON framework being added to the kernel, phy-omap-control > shouldn't be needed and it also provides a uniform API across all the > modules to program the control module. Ok, so the "phy-control" devices were really just a few registers of a system controller device that does a lot of other things as well, right? Can you put your description above into the cover-letter for the series, and the merge commit? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] arm: boot: store ATAGs structure into DT "/chosen/linux,atags" entry
On Tuesday 15 December 2015 10:33:25 Pali Rohár wrote: > On Monday 30 November 2015 11:09:42 Nicolas Pitre wrote: > > On Mon, 30 Nov 2015, Pali Rohár wrote: > > > On Monday 30 November 2015 07:23:53 Tony Lindgren wrote: > > > > * Pali Rohár [151129 16:16]: > > > > > On Monday 30 November 2015 01:09:17 Nicolas Pitre wrote: > > > > > > On Sun, 29 Nov 2015, Russell King - ARM Linux wrote: > > > In arch/arm/kernel/setup.c is function setup_arch() and it calls: > > > > > > mdesc = setup_machine_fdt(__atags_pointer); > > > if (!mdesc) > > > mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type); > > > > > > So it looks like that on atags address is stored either atags structure > > > or DT structure... so it is truth kernel uncompress code put DT blob to > > > same offset where is expected atags structure? > > > > No. It doesn't put it anywhere. Those functions read DT/ATAGs from the > > passed address. But you know this address won't be the one you want for > > the legacy ATAGs. > > > > What you should do is to add a init_early hook to your mdesc structure > > and retrieve your ATAGs from there directly at PAGE_OFFSET + 0x100. > > > > Now I suspect paging_init() marks the point where the ATAGs will be > > overwritten. To prevent this, you might have to add an additional tweak > > in arm_mm_memblock_reserve() similar to the one already present for > > CONFIG_SA. Something like: > > > > memblock_reserve(PHYS_OFFSET, PAGE_SIZE); > > > > And later on you can return that page back to the system. > > > > So am I understand correctly that solution would be to hack > arch/arm/mm/mmu.c to not overwrite page at PHYS_OFFSET? I would think we can just copy the data from PAGE_OFFSET + 0x100 to a some other page from your init_early hook. IIRC you can't use kmalloc there, but memblock_alloc() should work. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] phy: use syscon framework APIs to set ctrl mod reg
On Tuesday 15 December 2015 14:45:59 Kishon Vijay Abraham I wrote: > This series is basically to deprecate using phy-omap-control and use > syscon APIs to program the control module registers. > > Changes from v2: > No changes. > > Changes from v1: > *) cleanup ti_pipe3_probe in multiple steps > *) other minor cleanups > > Changes from [1] in PHY patches include > *) cleanup ti_pipe3_probe > *) have mask, power_on and power_off values in usb_phy_data for >omap-usb2 phy > > The patches have been pushed to > git://git.ti.com/linux-phy/linux-phy.git syscon > > [1] -> https://lkml.org/lkml/2015/6/23/189 > > All the testing was done both before applying the dt patches and after > applying the dt patches (dt patches will be posted shortly). > Can you explain here what the conversion is good for? Why do you prefer the syscon mapping over a high-level driver in this case? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: renesas_usb3: add support for Renesas USB3.0 peripheral controller
On Tuesday 15 December 2015 15:54:32 Yoshihiro Shimoda wrote:
> R-Car H3 has USB3.0 peripheral controllers. This controller's has the
> following features:
> - Supports super, high and full speed
> - Contains 30 pipes for bulk or interrupt transfer
> - Contains dedicated DMA controller
>
> This driver doesn't support the dedicated DMAC for now.
>
> Signed-off-by: Yoshihiro Shimoda
> ---
> This patch is based on the latest Felipe's usb.git / testing/next branch.
> (commit id = e9284de9fae69f1d5e57a4817bfc36dc5f3adf71)
Looks good overall, I've found a few small details:
> .../devicetree/bindings/usb/renesas_usb3.txt | 23 +
> drivers/usb/gadget/udc/Kconfig | 11 +
> drivers/usb/gadget/udc/Makefile|1 +
> drivers/usb/gadget/udc/renesas_usb3.c | 1720
>
> drivers/usb/gadget/udc/renesas_usb3.h | 284
The header file is only used by one .c file, so just merge it all into that
file.
> +Required properties:
> + - compatible: Must contain one of the following:
> + - "renesas,usb3-peri-r8a7795"
> + - reg: Base address and length of the register for the USB3.0 Peripheral
> + - interrupts: Interrupt specifier for the USB3.0 Peripheral
> + - clocks: A list of phandle + clock specifier pairs
The clock specifier contains the phandle, please rephrase the last line
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> b/drivers/usb/gadget/udc/renesas_usb3.c
> new file mode 100644
> index 000..f302c89
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -0,0 +1,1720 @@
> +/*
> + * Renesas USB3.0 Peripheral driver (USB gadget)
> + *
> + * Copyright (C) 2015 Renesas Electronics Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
As the 0-say bot found, this needs #include
> +static int renesas_usb3_ep_queue(struct usb_ep *_ep, struct usb_request
> *_req,
> + gfp_t gfp_flags);
> +static void usb3_start_pipen(struct renesas_usb3_ep *usb3_ep,
> + struct renesas_usb3_request *usb3_req);
Can you try to reorder the functions so you don't need forward declarations?
> +static void usb3_write(struct renesas_usb3 *usb3, u32 data, u32 offs)
> +{
> + iowrite32(data, usb3->reg + offs);
> +}
> +
> +static u32 usb3_read(struct renesas_usb3 *usb3, u32 offs)
> +{
> + return ioread32(usb3->reg + offs);
> +}
I think using readl() is more common than ioread32() if the driver cannot
use IORESOURCE_IO but only IORESOURCE_MEM.
On ARM, the two are the same, but on some architectures ioread32 is more
expensive, so using the former is preferred.
> + for (i = 0; i < USB3_WAIT_NS; i++) {
> + if ((usb3_read(usb3, reg) & mask) == expected)
> + return 0;
> + ndelay(1);
> + }
ndelay(1) is unusual, maybe use cpu_relax() instead, or document why
you call ndelay()?
> +static void usb3_init_phy(struct renesas_usb3 *usb3)
> +{
> +}
If the function is empty, don't add or call it, it's easy to add if you
need it later.
> +static struct platform_driver renesas_usb3_driver = {
> + .remove = __exit_p(renesas_usb3_remove),
> + .driver = {
> + .name = (char *)udc_name,
> + .of_match_table = of_match_ptr(usb3_of_match),
> + },
> +};
> +module_platform_driver_probe(renesas_usb3_driver, renesas_usb3_probe);
module_platform_driver_probe() won't work if you get into deferred probing,
I'd suggest using module_platform_driver() and not marking the remove
function as __exit
> +struct renesas_usb3;
> +struct renesas_usb3_request {
> + struct usb_request req;
> + struct list_headqueue;
> +};
There is already a list_head in struct usb_request. Could you use that?
(I don't know, just asking because this looks odd)
> +#define USB3_EP_NAME_SIZE8
> +struct renesas_usb3_ep {
> + struct usb_ep ep;
> + struct renesas_usb3 *usb3;
> + int num;
> + char ep_name[USB3_EP_NAME_SIZE];
> + struct list_head queue;
> + u32 rammap_val;
> + bool dir_in;
> + bool halt;
> + bool wedge;
> + bool started;
> +};
> +
> +struct renesas_usb3_priv {
> + int ramsize_per_ramif; /* unit = bytes */
> + int num_ramif;
> + int ramsize_per_pipe; /* unit = bytes */
> + unsigned workaround_for_vbus:1; /* if 1, don't check vbus signal */
> +};
You use 'bool' in one structure, and bit fields in the other.
Maybe pick one of the two styles consistently.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to ma
Re: [PATCH v3 3/5] soc: rockchip: add reboot notifier driver
On Wednesday 18 November 2015 17:56:22 Andy Yan wrote:
> rockchip platform have a protocol to pass the kernel reboot
> mode to bootloader by some special registers when system reboot.
> By this way the bootloader can take different action according
> to the different kernel reboot mode, for example, command
> "reboot loader" will reboot the board to rockusb mode, this is
> a very convenient way to get the board enter download mode.
>
> Signed-off-by: Andy Yan
Adding John Stultz to Cc
I just saw this thread pop up again, and had to think of John's recent
patch to unify this across platforms.
John, can you have a look at this driver too, and see how it fits in?
I think this is yet another variant, using an MMIO register rather than
RAM (as HTC / NVIDIA does) or SRAM (as Qualcomm does), but otherwise
it conceptually fits in with what you had.
The driver goes through an existing syscon regmap as far as I can
tell, rather than a memory area or its own device.
Arnd
> ---
>
> Changes in v3:
> - move from mach-rockchip to drivers/soc/rockchip, as the tegra does
> - use dts pass the related register
>
> Changes in v2:
> - check cpu dt node
> - remove a unnecessary of_put_node in function rockchip_get_pmu_regmap
> - fix a align issue
> - use reboot_notifier instead of restart_handler
>
> drivers/soc/rockchip/Kconfig | 7
> drivers/soc/rockchip/Makefile | 1 +
> drivers/soc/rockchip/loader.h | 22 ++
> drivers/soc/rockchip/reboot.c | 98
> +++
> 4 files changed, 128 insertions(+)
> create mode 100644 drivers/soc/rockchip/loader.h
> create mode 100644 drivers/soc/rockchip/reboot.c
>
> diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig
> index 7140ff8..4edbc44 100644
> --- a/drivers/soc/rockchip/Kconfig
> +++ b/drivers/soc/rockchip/Kconfig
> @@ -15,4 +15,11 @@ config ROCKCHIP_PM_DOMAINS
>
>If unsure, say N.
>
> +config ROCKCHIP_REBOOT
> + bool "Rockchip reboot notifier driver"
> + help
> + Say y here will enable reboot notifier support.
> + This will get reboot mode arguments from userspace and
> + store it in special register.
> +
> endif
> diff --git a/drivers/soc/rockchip/Makefile b/drivers/soc/rockchip/Makefile
> index 3d73d06..9817496 100644
> --- a/drivers/soc/rockchip/Makefile
> +++ b/drivers/soc/rockchip/Makefile
> @@ -2,3 +2,4 @@
> # Rockchip Soc drivers
> #
> obj-$(CONFIG_ROCKCHIP_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_ROCKCHIP_REBOOT) += reboot.o
> diff --git a/drivers/soc/rockchip/loader.h b/drivers/soc/rockchip/loader.h
> new file mode 100644
> index 000..bf51baa
> --- /dev/null
> +++ b/drivers/soc/rockchip/loader.h
> @@ -0,0 +1,22 @@
> +#ifndef __MACH_ROCKCHIP_LOADER_H
> +#define __MACH_ROCKCHIP_LOADER_H
> +
> +/*high 24 bits is tag, low 8 bits is type*/
> +#define SYS_LOADER_REBOOT_FLAG 0x5242C300
> +
> +enum {
> + BOOT_NORMAL = 0, /* normal boot */
> + BOOT_LOADER, /* enter loader rockusb mode */
> + BOOT_MASKROM,/* enter maskrom rockusb mode (not support now) */
> + BOOT_RECOVER,/* enter recover */
> + BOOT_NORECOVER, /* do not enter recover */
> + BOOT_SECONDOS, /* boot second OS (not support now)*/
> + BOOT_WIPEDATA, /* enter recover and wipe data. */
> + BOOT_WIPEALL,/* enter recover and wipe all data. */
> + BOOT_CHECKIMG, /* check firmware img with backup part*/
> + BOOT_FASTBOOT, /* enter fast boot mode */
> + BOOT_SECUREBOOT_DISABLE,
> + BOOT_CHARGING, /* enter charge mode */
> + BOOT_MAX /* MAX VALID BOOT TYPE.*/
> +};
> +#endif
> diff --git a/drivers/soc/rockchip/reboot.c b/drivers/soc/rockchip/reboot.c
> new file mode 100644
> index 000..048aeb0b
> --- /dev/null
> +++ b/drivers/soc/rockchip/reboot.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "loader.h"
> +
> +struct rockchip_reboot {
> + struct device *dev;
> + struct regmap *map;
> + u32 offset;
> + struct notifier_block reboot_notifier;
> +};
> +
> +static void rockchip_get_reboot_flag(const char *cmd, u32 *flag)
> +{
> + *flag = SYS_LOADER_REBOOT_FLAG + BOOT_NORMAL;
> +
> + if (cmd) {
> + if (!strcmp(cmd, "loader") || !strcmp(cmd, "bootloader"))
> + *flag = SYS_LOADER_REBOOT_FLAG + BOOT_LOADER;
> + else if (!strcmp(cmd, "recovery"))
> + *flag = SYS_LOADER_REBOOT_FLAG + BOOT_RECOVER;
> + else if (!strcmp(cmd, "charge"))
> + *
Re: [PATCH v2 0/3] USB: add generic onboard USB HUB driver
On Monday 14 December 2015 15:26:11 Peter Chen wrote: > Hi all, > > There is a known issue that the USB code can't handle USB HUB's > external pins well, in that case, it may cause some onboard > USB HUBs can't work since their PHY's clock or reset pin needs to > operate. > > The user reported this issue at below: > http://www.spinics.net/lists/linux-usb/msg131502.html > > In this patch set, I add a generic onboard USB HUB driver to > handle this problem, the external signals will be configured > before usb controller's initialization, it much likes we did > it at board code before. > > The user needs to add this generic hub node at dts to support it. > > @The udoo users, help to test please. I still think need to do this properly by representing the hub device as a USB device, using power sequencing code like MMC does. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] arm,arm64: uniphier: add a new driver, device tree updates
On Tuesday 24 November 2015 18:39:18 Masahiro Yamada wrote:
>
> Here is another series for UniPhier SoC family:
>
> - 1/4: add a new driver. The UniPhier System Bus is an external bus
> where on-board devices are connected to the SoC.
> (please check if the binding specification is OK.)
>
> - 2/4: device tree updates to use the driver added by 1/4
>
> - 3/4: trivial rename of device node names
>
> - 4/4: initial commit for ARMv8 SoC/Board device trees
>
> Please apply 2/4, 3/4/, 4/4 into the same branch because 4/4 depends on 2/4
> and 3/4.
> (4/4 creates symbolic links to ARM device trees.)
I'm a bit confused how this relates to the newer version ("[PATCH v5] bus:
uniphier-system-bus: add UniPhier System Bus driver"). The new version
only has one patch instead of four, so I'm not sure if the patches 2, 3
and 4 of this series still apply.
Can you clarify, or send the entire series again?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] ARM: dts: uniphier: clean up DTSIs by factoring the common parts out
On Thursday 03 December 2015 15:33:55 Masahiro Yamada wrote: > Masahiro Yamada (2): > ARM: dts: uniphier: change IRQ number of UART3 of PH1-Pro4 SoC > ARM: dts: uniphier: factor out common nodes to uniphier-common32.dtsi > Applied both to next/dt, thanks! Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MAINTAINERS: Add missing platform maintainers for dts files
On Thursday 10 December 2015 17:38:23 Rob Herring wrote: > Platform dts files need to be reviewed primarily by the platform > maintainers as dts files typically go in thru their trees. Add the missing > paths where there are existing maintainers listed. > > Signed-off-by: Rob Herring > Acked-by: Arnd Bergmann Do you want to collect the Acks and take this through your git tree, so should we pick it up into arm-soc? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] misc: Introduce reboot_reason driver
On Thursday 10 December 2015 13:43:16 John Stultz wrote: > On Thu, Dec 10, 2015 at 12:24 PM, Rob Herring wrote: > > The fact that we are using notifiers for reset reason and triggering > > is probably some indication that some infrastructure is needed. But I > > don't think you need to do that here as long as it is all kernel > > internals. We'll make the 2nd guy do it. > > > > Though, just so I understand better, what is problematic w/ the reset > notifiers? They provide the reboot command argument, which is the core > of what is needed. It actually seemed like it was almost designed with > this problem in mind. Notifiers in general are a bit of a kludge. We often use them in places that have not been abstracted well enough yet, and they make it less obvious what is actually going on when something happens, or in what order things are called. I'm actually less worried about the notifier side here than about the general problem of the communication channel. The reboot reason is only one of a number of things that the kernel needs to communicate to the boot loader. Other things may include: - boot device - location of the kernel - command line - properties of the /chosen DT node in general - boot scripts - ethernet MAC addresses - bootloader console configuration Every bootloader is different here regarding what can be configured and how we do it. Often the configuration is done entirely from user space, but some platforms have kernel support. So picking one particular aspect and trying to unify that one with a common kernel interface but ignoring all the others may cause problems if we later want to add a more general abstraction. It also looks like at least some of the interfaces require a checksum to be updated, or are based on variable-length entries, both of which require a proper driver. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] misc: Introduce reboot_reason driver
On Wednesday 09 December 2015 17:19:52 John Stultz wrote: > On Wed, Dec 9, 2015 at 2:07 AM, Arnd Bergmann wrote: > > On Tuesday 08 December 2015 16:22:40 John Stultz wrote: > >> >> diff --git a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts > >> >> b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts > > > > If the two known implementations are already fundamentally different, > > there is a good chance that other vendors have found some more ways > > to do the same thing, so we might need to do this as a framework, > > or merge it into an existing framework. > > > > Maybe it can be done in the same driver that also handles rebooting > > the machine? Those are typically in drivers/power/reset or > > in drivers/watchdog/ for machines that use the watchdog as the only > > way to reboot the machine. We can have additional device specific > > properties along with the reboot method (e.g. a reference to the > > SRAM or some syscon node) and add common code in another file if > > we need it. > > Heh. So my original patch to support this was actually tied into the > ps_hold reboot logic in the pinctrl-msm code: > https://git.linaro.org/people/john.stultz/flo.git/commitdiff/55f28281306af2cc6c61aa001030cb90da096ffa?hp=ad39413ecde7acd39c1f017249b1443ce4ef6812 > > But realizing I'd like to support this same feature on other hardware, > and we'd be duplicating the logic over and over for each device/reboot > method, it seemed having a fairly generic driver would be better. > > Looking at a few other devices, I saw one example that wanted both a > string and a value at the same time, so I probably could extend the > driver to have both reason strings and values, and would set which > ever (or both) were specified. That would cover all the cases I've > seen except the UEFI methods. > > (Though I suspect I still have to wrap my head more around the DT > bindings side of things) The problem is actually something else: from the two machines we looked at, it's clear that the reboot-reason is not actually something that is hardware specific, but instead depends on the bootloader. HTC has its own loader for Tegra, so we can't put the implementation into the Tegra reboot driver because that wouldn't work on non-HTC machines, and conversely, another device from HTC that uses a Qualcomm chip might use the machine vendor specific method rather than the SoC vendor designed one. There are actually tons of different ways to do the same thing, including the PC nvram, the Nokia N900 method that has been discussed to no end because it relies on ATAGS, IBM has a key-value pair method for open firmware using NVRAM, and Broadcom uses their own layout on a number of different devices (nvram, eeprom, NOR-flash, NAND-flash). > >> >> + /* initialize specified reasons from DT */ > >> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,none", &val)) > >> >> + reasons[NONE] = val; > >> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,bootloader", > >> >> &val)) > >> >> + reasons[BOOTLOADER] = val; > >> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,recovery", > >> >> &val)) > >> >> + reasons[RECOVERY] = val; > >> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,oem", &val)) > >> >> + reasons[OEM] = val; > >> > > >> > I would like for this to be less hard coded. > >> > >> Any tips here on how to do so? > > > > If we move this logic into the qcom reset driver in > > drivers/power/reset/msm-poweroff.c, we should be good. > > If the concern is that since DT is basically ABI, one might not want > to have such a wide interface that specifies all the different > reasons, I can understand that. Though I'm really not sure how else we > would be able to specify the device supported the reboot reason logic > w/o having something in the DT (since some device may use the same soc > w/ the same reboot logic may use a different bootloader which doesn't > support the reason methods). At that point if we don't describe the > method clearly, it ends up being something closer to just a quirks > list which we'd have to map internally to behavior, which doesn't seem > great. > > Should we run into hardware that the proposed driver doesn't handle, > we can introduce a new driver for those specific semantics, but this > way we can share at least most of the logic, no? I think we need a layered approach, with some high-level code to store the boot reason, but then support firmware specific backends to that. If we just need a phandle for an SRAM partition and an offset within it, that can be done by the high-level driver, but not any of the more sophisticated communication methods. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: l2x0: make it possible to disable outer sync from DT
On Thursday 10 December 2015 15:14:15 Linus Walleij wrote: > Some RealView platforms have broken outer_sync, see: > http://marc.info/?l=linux-kernel&m=144846940516899&w=2 > > We got rid of the custom barriers from the machine by disabling > outer sync, but that was just for the boardfile case. We have > to be able to do the same in the device tree case. > > Since __l2c_init() is cloning and copying the L2C vtable, > we pass an argument to this function to optionally numb > the outer sync operation if desired, before initializing > the cache. > > After this we can set up the cache correctly on the RealView > PB11MPCore, and it boots rock solid with the cache enabled. > Before this, spurious crashes would occur if we try to set > up the cache properly. > > Cc: Russell King > Cc: Arnd Bergmann > Cc: devicetree@vger.kernel.org > Signed-off-by: Linus Walleij This has the same effect as my "ARM: realview: remove private barrier implementation" patch and replaces part of that, correct? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/9] ARM: dts: add dts files for hi3519-demb board
On Thursday 10 December 2015 16:59:14 xuejiancheng wrote: > On 2015/12/10 16:04, Arnd Bergmann wrote: > > On Thursday 10 December 2015 14:32:05 xuejiancheng wrote: > >>> > >>> This is not what I meant. You have to use "syscon" as the most generic > >>> "compatible" value here, but should add a machine specific string > >>> as a more specific one. "hisilicon,sysctrl" is not appropriate because > >>> it does not identify the IP block uniquely, you can only use that > >>> in combination with another more specific string. > >> > >> OK. I will use "hisilicon,hi3519-syscon" and "syscon" as the compatible > >> value > >> for the sysctrl node in hi3519.dtsi. > >> > > > > Why hisilicon,hi3519-syscon instead of hisilicon,hi3519-sysctrl? > > > Is this not called a sysctrl at all in the datasheet then? > > It is OK to use hisilicon,hi3519-sysctrl. > > I thought syscon was more commonly used as abbr. of system-controller in the > kernel code. > > "syscon" is the generic name for the framework in the kernel. This has nothing to do with how chip designers call their devices, and the DT should always match what the datasheet says, not what a random operating system calls things, even if it's the only OS you care about. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] misc: Introduce reboot_reason driver
On Wednesday 09 December 2015 17:32:02 John Stultz wrote:
> On Tue, Dec 8, 2015 at 2:07 PM, Bjorn Andersson
> wrote:
> > On Tue 08 Dec 13:29 PST 2015, John Stultz wrote:
> >> diff --git a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> index 5183d18..ee5dcb7 100644
> >> --- a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> +++ b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> @@ -282,6 +282,15 @@
> >> };
> >> };
> >>
> >> + reboot_reason: reboot_reason@2a03f65c {
> >> + compatible = "reboot_reason";
> >> + reg = <0x2A03F65C 0x4>;
> >> + reason,none = <0x77665501>;
> >> + reason,bootloader = <0x77665500>;
> >> + reason,recovery = <0x77665502>;
> >> + reason,oem = <0x6f656d00>;
> >> + };
> >> +
> >
> > This address refers to IMEM, which is shared with a number of other
> > uses. So I think we should have a simple-mfd (and syscon) with this
> > within.
>
> So talking with Arnd some more it looked like IMEM was really just
> SRAM. Is that not the case, or is there something else special about
> it? Does it really need simple-mfd and syscon? I'm still fuzzy on how
> to use those for this.
If it's SRAM, we should use the SRAM binding and not make it a syscon
device. What we can have however, is a mostly somewhat reboot-reason
driver that is able to access an SRAM device or something else,
depending on what the platform and/or bootloader has.
HTC's Nexus 9 apparently uses a section of normal RAM for communication
between bootloader and kernel, so we'd also need a way to hook into
a driver for that.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/9] ARM: dts: add dts files for hi3519-demb board
On Thursday 10 December 2015 14:32:05 xuejiancheng wrote: > > > > This is not what I meant. You have to use "syscon" as the most generic > > "compatible" value here, but should add a machine specific string > > as a more specific one. "hisilicon,sysctrl" is not appropriate because > > it does not identify the IP block uniquely, you can only use that > > in combination with another more specific string. > > OK. I will use "hisilicon,hi3519-syscon" and "syscon" as the compatible value > for the sysctrl node in hi3519.dtsi. > Why hisilicon,hi3519-syscon instead of hisilicon,hi3519-sysctrl? Is this not called a sysctrl at all in the datasheet then? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/9] ARM: dts: add dts files for hi3519-demb board
On Tuesday 08 December 2015 11:54:51 xuejiancheng wrote: > On 2015/12/7 14:37, xuejiancheng wrote: > > > > On 2015/12/4 18:49, Arnd Bergmann wrote: > >> On Friday 04 December 2015 10:27:58 xuejiancheng wrote: > >>>> > >> Maybe split out the sysctrl binding from > >> Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt, as it has > >> you already have a couple of those, and it's not clear how they relate > >> to one another. > >> > >> If we introduce a string for all hip04 compatible sysctrl devices, we > >> should > >> document that and use it consistently, so hi3519 becomes > >> > >> compatible = "hisilicon,hi3519-sysctrl", "hisilicon,hip04-sysctrl", > >> "hisilicon,sysctrl"; > >> > >> but I'd clarify in the binding documentation that "hisilicon,sysctrl" > >> should > >> only be used for hip04 and hi3519 but not the others. > >> > >> As this seems to be a standard part, we can also think about making a > >> high-level driver for in in drivers/soc rather than relying on the syscon > >> driver which we tend to use more for one-off devices with random register > >> layouts. > >> > >Sorry. I didn't understand your meaning well and maybe I gave you a > > wrong description. > > Please allow me to clarify it again. > >The "sysctrl" nodes here is just used for the "reboot" function. It is > > corresponding to > > the driver "drivers/power/reset/hisi-reboot.c". The compatible string in > > the driver is > > "hisilicon,sysctrl". > >The layout of this block is also different from the one in HiP04. > > I'll use "syscon" as the compatible value for sysctrl node and > "syscon-reboot" for a new reboot node. > > This is not what I meant. You have to use "syscon" as the most generic "compatible" value here, but should add a machine specific string as a more specific one. "hisilicon,sysctrl" is not appropriate because it does not identify the IP block uniquely, you can only use that in combination with another more specific string. That way, we have to option to create a high-level driver for the IP block later if it turns out that we need some more generic functionality that is provided by those registers. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] misc: Introduce reboot_reason driver
On Tuesday 08 December 2015 16:22:40 John Stultz wrote:
> >> diff --git a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> index 5183d18..ee5dcb7 100644
> >> --- a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> +++ b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> >> @@ -282,6 +282,15 @@
> >> };
> >> };
> >>
> >> + reboot_reason: reboot_reason@2a03f65c {
> >> + compatible = "reboot_reason";
> >> + reg = <0x2A03F65C 0x4>;
> >> + reason,none = <0x77665501>;
> >> + reason,bootloader = <0x77665500>;
> >> + reason,recovery = <0x77665502>;
> >> + reason,oem = <0x6f656d00>;
> >> + };
> >> +
> >
> > This address refers to IMEM, which is shared with a number of other
> > uses. So I think we should have a simple-mfd (and syscon) with this
> > within.
>
> Errr.. I'm going to have to read up there. I'm not super familiar with
> any of those drivers, so I'll try to see how exactly they would map in
> here.
Is this an SRAM? We already have a generic SRAM binding, and I think this
would fit in there.
> > I like the fact that you don't hard code the magics in the
> > implementation, as I've seen additions from multiple places - so perhaps
> > it should be made even more flexible.
> >
> > OMAP seems to use strings here instead of magics, but the delivery
> > mechanism looks to be the same. But I think of this as Qualcomm
> > specific.
>
> Yea. This is good feedback. EFI bootloaders have their own
> implementations as well. I suspect we'll need a few different driver
> "types" to handle these differences, ie: value vs string.
>
> I'll maybe extend the compatible string to make it clear that this is
> a "value" style, and we can extend it with a string type later if
> folks care?
If the two known implementations are already fundamentally different,
there is a good chance that other vendors have found some more ways
to do the same thing, so we might need to do this as a framework,
or merge it into an existing framework.
Maybe it can be done in the same driver that also handles rebooting
the machine? Those are typically in drivers/power/reset or
in drivers/watchdog/ for machines that use the watchdog as the only
way to reboot the machine. We can have additional device specific
properties along with the reboot method (e.g. a reference to the
SRAM or some syscon node) and add common code in another file if
we need it.
> >> + /* initialize specified reasons from DT */
> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,none", &val))
> >> + reasons[NONE] = val;
> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,bootloader",
> >> &val))
> >> + reasons[BOOTLOADER] = val;
> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,recovery",
> >> &val))
> >> + reasons[RECOVERY] = val;
> >> + if (!of_property_read_u32(pdev->dev.of_node, "reason,oem", &val))
> >> + reasons[OEM] = val;
> >
> > I would like for this to be less hard coded.
>
> Any tips here on how to do so?
If we move this logic into the qcom reset driver in
drivers/power/reset/msm-poweroff.c, we should be good.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: arRe: [PATCH net-next 2/2] net: hns: enet specisies a reference to dsaf (config and documents)
On Wednesday 09 December 2015 17:25:13 Yankejian wrote: > thanks a lot for pointing it out. > > It is great regret that this change breaks compatibility with old dtbs. > this is a new driver which is run on developing boards, and all the > clients' boards are developing boards. So we provide them a method to > update the firmware on the board, once we update the dtsi and kernel, > we require our clients to update the existed firmware and kernel. > Therefore, these changes is actually under control. Shall we treat this > by this way? Ok, if you can show that the incompatible change is safe to do, that's fine. Just put the explanation above into the patch description to document that you have considered the effects of the change, and to ensure that it gets done atomically. Also, you have to merge the two patches into one to allow bisection, or do a series of three patches that need to be applied in order: 1. add ae-handle property 2. change driver to use that property 3. remove ae-name property Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/5] PCI: designware: add memory barrier after enabling region
On Wednesday 09 December 2015 10:10:05 Pratyush Anand wrote: > On Tue, Dec 8, 2015 at 2:31 PM, Stanimir Varbanov > > > Signed-off-by: Stanimir Varbanov > > > --- > > > drivers/pci/host/pcie-designware.c |5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/pci/host/pcie-designware.c > > > b/drivers/pci/host/pcie-designware.c > > > index 02a7452bdf23..ed4dc2e2553b 100644 > > > --- a/drivers/pci/host/pcie-designware.c > > > +++ b/drivers/pci/host/pcie-designware.c > > > @@ -164,6 +164,11 @@ static void dw_pcie_prog_outbound_atu(struct > > > pcie_port *pp, int index, > > > dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), > > > PCIE_ATU_UPPER_TARGET); > > > dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1); > > > dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2); > > > + /* > > > + * ensure that the ATU enable has been happaned before accessing > > > + * pci configuration/io spaces through dw_pcie_cfg_[read|write]. > > > + */ > > > + wmb(); > > > } > > > > > > My understnading is that since writel() of dw_pcie_writel_rc() in > above code and readl(), writel() of dw_pcie_cfg_[read|write]() (which > will follow) goes through same device (ie PCIe host here). So, it is > guaranteed that 1st writel() will be executed before later > readl()/writel(). If that is true then we do not need any explicit > barrier here. > > Arnd, Russel: whats your opinion here. I think the ordering is only enforced if the two register accesses are on the same device as seen from the bus, and it's possible that the RC registers and the config space registers are not considered the same thing here. For config write, this is not a problem, because the config space write has a wmb() that enforces ordering, but it's possible that the config space read may hit the device in parallel with the PCIE_ATU_ENABLE write. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] stmmac: create of compatible mdio bus for stmacc driver
On Wednesday 09 December 2015 10:47:29 Phil Reid wrote: > On 9/12/2015 10:15 AM, kbuild test robot wrote: > > > > > url: > > https://github.com/0day-ci/linux/commits/Phil-Reid/stmmac-create-of-compatible-mdio-bus-for-stmacc-driver/20151209-094242 > > config: x86_64-randconfig-b0-12090825 (attached as .config) > > reproduce: > > # save the attached .config to linux build tree > > make ARCH=x86_64 > > > > All errors (new ones prefixed by >>): > > > > drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c: In function > > 'stmmac_mdio_register': > >>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c:226:17: error: 'struct > >>> stmmac_mdio_bus_data' has no member named 'reset_gpio' > > mdio_bus_data->reset_gpio = -1; > > ^ > > G'day Arnd, > > I got the above error from kbuild test robot. When changing to if > (IS_ENABLED(CONFIG_OF)). > This was existing code that I moved into this conditional. > reset_gpio member only exists when CONFIG_OF is defined. > So it looks like I need to move that code back to the #if. > unless you have an alternate suggestion. I think it would be slightly nicer to remove the #ifdef in the header file as well and always have those members, it's only a few bytes in any system that we are saving here and there are only two actual users of this driver that don't already require CONFIG_OF (blackfin ezkit and mips loongson32). If Giuseppe prefers to keep the #ifdef, going back to your previous version is fine with me too. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 08/11] ARM: STi: Register CPUFreq device
On Tuesday 08 December 2015 14:32:01 Lee Jones wrote:
> @@ -161,3 +166,11 @@ struct smp_operations __initdata sti_smp_ops = {
> .smp_secondary_init = sti_secondary_init,
> .smp_boot_secondary = sti_boot_secondary,
> };
> +
> +/**
> + * CPUFreq Registration
> + */
> +void init_cpufreq(void)
> +{
> + platform_device_register_simple("sti-cpufreq", -1, NULL, 0);
> +}
>
Can you please do this under drivers/cpufreq somewhere?
I really don't want to any more of these in platform code. Requiring a
device to be created just to probe the driver is really silly.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver
On Wednesday 09 December 2015 09:57:40 Lucas Stach wrote:
> Am Mittwoch, den 09.12.2015, 16:50 +0800 schrieb Peter Chen:
> > On Tue, Dec 08, 2015 at 08:36:00AM -0700, Mathieu Poirier wrote:
> > > On 7 December 2015 at 18:37, Peter Chen wrote:
> > > > +
> > > > + if (dev->of_node) {
> > > > + struct device_node *node = dev->of_node;
> > > > +
> > > > + hub_data->clk = devm_clk_get(dev, "external_clk");
> > > > + if (IS_ERR(hub_data->clk)) {
> > > > + dev_dbg(dev, "Can't get external clock: %ld\n",
> > > > + PTR_ERR(hub_data->clk));
> > > > + }
> > >
> > > Is the intended behaviour to keep going here event when there is an
> > > error? Can the "hub_data" really work without a clock?
> >
> > Yes, some HUB may work with fixed 24M OSC at the board, but they need to
> > reset through external IO, so the clock is not need at this case, but
> > reset pin is mandatory.
> >
> If the hub always requires a clock it must not be optional. If you have
> a fixed 24MHz clock on board add this to the DT as a fixed-clock and use
> it as an input to the hub.
I think it's fine to make the clock optional in the sense that you only
need to list non-fixed clocks that have to be enabled and or controlled.
Which reminds me, should the driver call clk_set_rate()? It currently
doesn't, but other machines might need that.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH for 4.4 0/2] DT/dmaengine: edma: Convert 16bit arrays to 32bit
On Wednesday 09 December 2015 10:18:09 Peter Ujfalusi wrote: > Hi, > > Based on the discussion regarding to (convert am33xx to use the new eDMA > bindings): > https://www.mail-archive.com/linux-omap@vger.kernel.org/msg122117.html > > This two patch will convert the new eDMA binding to not use 16bit arrays for > memcpy channel selection and for marking slots reserved. > The '/bits/ 16' seams to be causing confusion so it is probably better to just > use standard type for the arrays. > > The new bindings for the eDMA is introduced for 4.4 and we do not have users > of > it, which means that we can still change it w/o the risk of breaking anything > and we do not need to maintain the compatibility with 16bit arrays. > > The changes in the eDMA driver is local to the DT parsing and should not > conflict with other changes (like the filter function mapping support). Hrm, > there might be trivial conflict in the include/linux/platform_data/edma.h with > the "dmaengine 'universal' API". Both patches Acked-by: Arnd Bergmann > Tony, Arnd, Vinod: Can you agree on the practicalities on how these patches > are > going to be handled? I would like to send the updated am33xx/am437x conversion > for 4.5 based on these changes. I'd suggest you send a pull request with these two patches on top of 4.4-rc1, and then either Vinod or I send it to Linus, and you base your other changes on top of the same branch. I think Vinod's tree is slightly more fitting for the 4.4 merge, but if he prefers me to take them instead, I will. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB
On Wednesday 09 December 2015 16:12:24 Peter Chen wrote: > On Tue, Dec 08, 2015 at 09:24:03PM -0600, Rob Herring wrote: > > On Tue, Dec 08, 2015 at 10:58:48AM +0100, Arnd Bergmann wrote: > > > On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote: > > > > This something we don't have to define ad-hoc. The hub hangs off an USB > > > > controller, right? The "Open Firmware recommended practice: USB" > > > > document already describes how to represent USB devices in a generic > > > > manner: > > > > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps > > > > > > > > Is there a reason not to reuse this? > > > > > > > > The usb hub node would be a child of the usb controller node, and it > > > > could use > > > > compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */ > > > > > > Good point, I had not thought of that when I looked at the patches. > > > > > > Yes, let's do this way. I don't know if we ever implemented the simple > > > patch to associate a USB device with a device_node, but if not, then > > > let's do it now for this driver. A lot of people have asked for it in > > > the past. > > > > Agreed. Also, some hubs have I2C buses as well, but I still think under > > the USB bus is the right place. > > > > However, one complication here is often (probably this case) these > > addtional signals need to be controlled before the device enumerates. > > > > Yes, I did not find a way to let the USB bus code handle it, so I had to > write a platform driver to do it Looping in Ulf, he solved the same problem for SDIO devices recently, and probably remembers the details best. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH] misc: Introduce reboot_reason driver
On Tuesday 08 December 2015 13:29:22 John Stultz wrote:
> diff --git a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> index 5183d18..ee5dcb7 100644
> --- a/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> +++ b/arch/arm/boot/dts/qcom-apq8064-nexus7-flo.dts
> @@ -282,6 +282,15 @@
> };
> };
>
> + reboot_reason: reboot_reason@2a03f65c {
> + compatible = "reboot_reason";
This is not a good compatible string. There should generally be a vendor
name associated with it (use "linux," if nothing else, and you should have
'-' instead of '_'.
> + reg = <0x2A03F65C 0x4>;
This may easily conflict with the device it is part of. We should have
non-overlapping register areas in general. For the example you are
looking at, which register block is this?
> +
> +/* Types of reasons */
> +static enum {
> + NONE,
> + BOOTLOADER,
> + RECOVERY,
> + OEM,
> + MAX_REASONS
> +} __maybe_unused reason_types;
The variable seems to always be unused, not just "__maybe_unused". Maybe remove
it?
> +static int reboot_reason(struct notifier_block *nb, unsigned long action,
> + void *data)
> +{
> + char *cmd = (char *)data;
> + long reason = reasons[NONE];
> +
> + if (!reboot_reason_addr)
> + return NOTIFY_DONE;
> +
> + if (cmd != NULL) {
> + if (!strncmp(cmd, "bootloader", 10))
> + reason = reasons[BOOTLOADER];
> + else if (!strncmp(cmd, "recovery", 8))
> + reason = reasons[RECOVERY];
> + else if (!strncmp(cmd, "oem-", 4)) {
> + unsigned long code;
> +
> + if (!kstrtoul(cmd+4, 0, &code))
> + reason = reasons[OEM] | (code & 0xff);
> + }
> + }
> +
> + if (reason != -1)
> + writel(reason, reboot_reason_addr);
> + return NOTIFY_DONE;
> +}
Will this reboot the machine?
> + /* Install the notifier */
> + restart_nb.notifier_call = reboot_reason;
> + restart_nb.priority = 256;
> + if (register_restart_handler(&restart_nb)) {
If not, you should use register_reboot_notifier() rather than
register_restart_handler(). The former gets called to do something
just before rebooting, while the latter attempts to actually reboot
the machine.
> +static int __init reboot_reason_init(void)
> +{
> + return platform_driver_register(&reboot_reason_driver);
> +}
> +arch_initcall(reboot_reason_init);
Why this early? If it can be a normal device_initcall, you can use
module_platform_driver().
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] bus: uniphier-system-bus: add UniPhier System Bus driver
On Wednesday 09 December 2015 01:21:58 Masahiro Yamada wrote:
> The UniPhier System Bus is an external that connects on-board devices
> to the UniPhier SoC. Each bank (chip select) is dynamically mapped
> to the CPU-viewed address base via the bus controller. The bus
> controller must be configured before any access to the bus.
>
> This driver parses the "ranges" property of the System Bus node and
> initialized the bus controller. After the bus becomes ready, devices
> below it are populated.
>
> Note:
> Each bank can be mapped anywhere in the supported address space;
> there is nothing preventing us from assigning bank 0 on 0x4200,
> 0x4300, or anywhere as long as such region is not used by others.
> So, the "ranges" is just one possible software configuration, which
> does not seem to fit in device tree because device tree is a hardware
> description language. However, of_translate_address() requires
> "ranges" in every bus node between CPUs and device mapped on the CPU
> address space. In other words, "ranges" properties must be statically
> defined in device tree. After some discussion, I decided the dynamic
> address reassignment by the driver is too bothersome. Instead, the
> device tree should provide a reasonable translation setup that the OS
> can rely on.
>
> Signed-off-by: Masahiro Yamada
> Acked-by: Rob Herring
Looks very nice.
Acked-by: Arnd Bergmann
Just a little thought about one thing I found odd:
> +static int uniphier_system_bus_check_overlap(
> + struct uniphier_system_bus_priv tmp)
> +{
Did you intentionally pass this by value? Maybe do it explicitly using a pointer
and memcpy to a local variable, which has a similar effect. Alternatively
just check each newly probed child node for conflicts with any of the
previous children. That is slightly more expensive at O(n^2) instead of O(n)
but n is always small here and you can avoid sorting first.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 net-next 2/3] net: ethernet: cadence-macb: Add fallback to read DT provided caps
On Tuesday 08 December 2015 14:52:05 Neil Armstrong wrote: > Add 1:1 mapping of software defines caps parsing from DT in case the > generic macb compatible form is used. > These properties will provide support for futures implementations > only defined from DT without need to update the driver code to support > new variants. > > Signed-off-by: Neil Armstrong > Translating the Linux implementation specific configuration into DT properties directly is usually not the best way. Could we instead have a lookup table by compatible string to set the flags? It seems that there are lots of different flags but only a couple of different users of this IP block. Also, the fact that you are now adding yet another quirk tells me that the set you define today is unlikely to cover all the future requirements. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: DTS: am33xx: Use the new DT bindings for the eDMA3
On Tuesday 08 December 2015 12:22:09 Peter Ujfalusi wrote: > On 12/08/2015 11:51 AM, Arnd Bergmann wrote: > > On Tuesday 08 December 2015 09:42:26 Peter Ujfalusi wrote: > >> On 12/04/2015 11:51 PM, Tony Lindgren wrote: > >>>> > >>>> Please just drop the /bits/ 16 and use normal cells. > >>> > >>> Yeah agreed, makes things less confusing for sure > >> > >> 4.4 will be the first kernel where we will have the new eDMA bindings. I > >> have > >> chosen to use 16bit array for specifying the channels used for memcpy > >> (ti,edma-memcpy-channels) and for the reserving paRAM slots > >> (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels > >> and > >> 512 paRAM slots. 16bit is more than enough to store this information and it > >> even gives us enough room if ever in the future these numbers are going to > >> increase (which they are not). > >> > >> But in order to change them to 32bit the driver needs to be changed as > >> well. > >> Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is > >> not > >> yet out, so it might be possible to change the binding document and the > >> driver > >> to use 32bit arrays. The driver internally uses 16bit type for these which > >> I'm > >> not going to change, but the code parsing the DT needs to be adjusted for > >> the > >> new data type. > >> > >> If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I > >> can cook up the patch(es) to do so. > > > > I hadn't realized that it was already in 4.4-rc. The change should be > > trivial > > enough though, so I'd still do it. If Vinod would rather not change it now, > > it's not overly important though. > > But this change must be done before we have actual users of these properties, > which is the am33xx, am437x and the da850 conversion series I have sent > recently. > We might want to have this changed for 4.4 since it is going to be an LTS > release... Yes, that's what I meant: We either get the patch into 4.4 by creating a branch for Vinod to pull with this change, and base all other changes in your 4.5 series on the same branch, or we don't change it at all. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] clk: hi3519: add dt-binding document and header file
On Tuesday 08 December 2015 17:45:25 xuejiancheng wrote: > On 2015/12/7 17:36, Arnd Bergmann wrote: > > On Monday 07 December 2015 16:01:03 xuejiancheng wrote: > >> On 2015/12/4 18:56, Arnd Bergmann wrote: > >>> On Friday 04 December 2015 11:21:28 xuejiancheng wrote: > >>>> Hi Arnd, > >>>> > >>>> On 2015/12/3 17:44, Arnd Bergmann wrote: > >>>>> On Thursday 03 December 2015 10:39:24 Jiancheng Xue wrote: > >>>>>> +#ifndef __DTS_HI3519_CLOCK_H > >>>>>> +#define __DTS_HI3519_CLOCK_H > >>>>> > >>>>> Please try to avoid adding headers like this if you can at all. > >>>>> > >>>>> I might ask you to merge the header file in one merge window > >>>>> otherwise and submit the platform code one kernel later, as they > >>>>> tendn to cause us needless dependencies otherwise. > >>>>> > >>>> > >>>> Sorry. In v1, Rob suggested putting binding doc and header files in > >>>> a separate patch. The clock driver indeed depends on the header. > >>>> > >>>> I will put the header and the clock driver in a patch, and keep the > >>>> binding doc in another patch. > >>> > >>> Having split patches is better, I was really commenting on the fact > >>> that ideally you would not have a header file at all. If we merge > >>> the header through arm-soc, then you won't be able to merge the > >>> clk driver easily, and if you merge the header through the clk > >>> maintainer, I'm can't take your dts files. > >> > >> Thank you for your comments. Because the clocks in the crg module have > >> different types and random layouts. If this header file is removed, > >> the clock driver and the dts files will get very complicated. > >> > >> Could you help me acknowledge it if I put the header file and clock driver > >> in a patch? > >> > >> Could you give me some suggestions If I want to keep this header file? > > > > If this is another clock controller that has a random register layout, > > then adding the header file is the least problematic solution indeed. > > Is it OK if I put the header file and the clock driver in a patch? > > If it's not OK, could you tell me how should I separate the patches? It's ok to do it like this, but then I can't easily merge any DT changes based on the header file into the arm-soc tree in the same merge window. Staging out the .dts files by one merge window is the easiest solution here, otherwise you need to set up a shared branch with the headers changes and base both the clk driver and the dts branch on top of that and cannot rebase those patches. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/6] mfd: syscon: add a DT property to set value width
On Monday 07 December 2015 14:42:18 Damien Riegel wrote: > Good to see this patch applied. What's going on now with the other > patches of this serie ? How should I handle them ? I don't see any build-time dependencies between the patches, so please submit them to the respective maintainers. Patch 3 and 5 should go through the watchdog tree, and Shawn can take the other ones and submit them through arm-soc. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB
On Tuesday 08 December 2015 10:50:49 Philipp Zabel wrote: > Am Dienstag, den 08.12.2015, 09:37 +0800 schrieb Peter Chen: > > Add dt-binding documentation for generic onboard USB HUB. > > > > Signed-off-by: Peter Chen > > --- > > .../bindings/usb/generic-onboard-hub.txt | 28 > > ++ > > 1 file changed, 28 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/usb/generic-onboard-hub.txt > > > > diff --git a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt > > b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt > > new file mode 100644 > > index 000..ea92205 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt > > @@ -0,0 +1,28 @@ > > +Generic Onboard USB HUB > >+ > > +Required properties: > > +- compatible: should be "generic-onboard-hub" > > This something we don't have to define ad-hoc. The hub hangs off an USB > controller, right? The "Open Firmware recommended practice: USB" > document already describes how to represent USB devices in a generic > manner: > http://www.firmware.org/1275/bindings/usb/usb-1_0.ps > > Is there a reason not to reuse this? > > The usb hub node would be a child of the usb controller node, and it > could use > compatible = "usb,class9"; /* bDeviceClass 9 (Hub) */ Good point, I had not thought of that when I looked at the patches. Yes, let's do this way. I don't know if we ever implemented the simple patch to associate a USB device with a device_node, but if not, then let's do it now for this driver. A lot of people have asked for it in the past. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: DTS: am33xx: Use the new DT bindings for the eDMA3
On Tuesday 08 December 2015 09:42:26 Peter Ujfalusi wrote: > On 12/04/2015 11:51 PM, Tony Lindgren wrote: > >> > >> Please just drop the /bits/ 16 and use normal cells. > > > > Yeah agreed, makes things less confusing for sure > > 4.4 will be the first kernel where we will have the new eDMA bindings. I have > chosen to use 16bit array for specifying the channels used for memcpy > (ti,edma-memcpy-channels) and for the reserving paRAM slots > (ti,edma-reserved-slot-ranges). As of now we have maximum of 64 channels and > 512 paRAM slots. 16bit is more than enough to store this information and it > even gives us enough room if ever in the future these numbers are going to > increase (which they are not). > > But in order to change them to 32bit the driver needs to be changed as well. > Currently we do not have drivers (in 4.4) using the new bindings, 4.4 is not > yet out, so it might be possible to change the binding document and the driver > to use 32bit arrays. The driver internally uses 16bit type for these which I'm > not going to change, but the code parsing the DT needs to be adjusted for the > new data type. > > If Vinod is willing to take update for the DT binding of eDMA for 4.4-rc, I > can cook up the patch(es) to do so. I hadn't realized that it was already in 4.4-rc. The change should be trivial enough though, so I'd still do it. If Vinod would rather not change it now, it's not overly important though. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: misc: generic_onboard_hub: add generic onboard USB HUB driver
On Tuesday 08 December 2015 09:37:48 Peter Chen wrote:
> +struct usb_hub_generic_data {
> + struct clk *clk;
> +};
> +
> +static int usb_hub_generic_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct usb_hub_generic_platform_data *pdata = dev->platform_data;
> + struct usb_hub_generic_data *hub_data;
> + int reset_pol = 0, duration_us = 50, ret = 0;
> + struct gpio_desc *gpiod_reset = NULL;
> +
> + hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> + if (!hub_data)
> + return -ENOMEM;
> +
> + if (dev->of_node) {
Let's not worry about the !DT case until someone adds a board file
that needs it. Just remove the if() here along and the whole else
block.
> +#ifdef CONFIG_OF
> +static const struct of_device_id usb_hub_generic_dt_ids[] = {
> + {.compatible = "generic-onboard-hub"},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, usb_hub_generic_dt_ids);
> +#endif
> +
> +static struct platform_driver usb_hub_generic_driver = {
> + .probe = usb_hub_generic_probe,
> + .remove = usb_hub_generic_remove,
> + .driver = {
> + .name = "usb_hub_generic_onboard",
> + .of_match_table = usb_hub_generic_dt_ids,
> + },
> +};
Build error when CONFIG_OF is disabled: Please remove the #ifdef around the
device
table.
> diff --git a/include/linux/usb/generic_onboard_hub.h
> b/include/linux/usb/generic_onboard_hub.h
> new file mode 100644
> index 000..1b70ccc
> --- /dev/null
> +++ b/include/linux/usb/generic_onboard_hub.h
> @@ -0,0 +1,11 @@
> +#ifndef __LINUX_USB_GENERIC_HUB_H
> +#define __LINUX_USB_GENERIC_HUB_H
> +
> +struct usb_hub_generic_platform_data {
> + int gpio_reset;
> + int gpio_reset_polarity;
> + int gpio_reset_duration_us;
> + struct clk *ext_clk;
> +};
Merge this structure into struct usb_hub_generic_data and remove the header.
ARnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] doc: dt-binding: generic onboard USB HUB
On Tuesday 08 December 2015 17:20:46 Peter Chen wrote:
> On Tue, Dec 08, 2015 at 12:30:59AM -0200, Fabio Estevam wrote:
> > On Mon, Dec 7, 2015 at 11:37 PM, Peter Chen
> > wrote:
> > > Add dt-binding documentation for generic onboard USB HUB.
> > >
> > > Signed-off-by: Peter Chen
> > > ---
> > > .../bindings/usb/generic-onboard-hub.txt | 28
> > > ++
> > > 1 file changed, 28 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > new file mode 100644
> > > index 000..ea92205
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/generic-onboard-hub.txt
> > > @@ -0,0 +1,28 @@
> > > +Generic Onboard USB HUB
> > > +
> > > +Required properties:
> > > +- compatible: should be "generic-onboard-hub"
> > > +
> > > +Optional properties:
> > > +- clocks: the input clock for HUB.
> > > +
> > > +- clock-names: Should be "external_clk"
Make this just "external", or remove the name and use it as an anonymous clock.
> > > +- hub-reset-gpios: Should specify the GPIO for reset.
> > > +
> > > +- hub-reset-active-high: the active reset signal is high, if this
> > > property is
> > > + not set, the active reset signal is low.
> > > +
> > > +- hub-reset-duration-us: the duration for assert reset signal, the time
> > > unit
> > > + is microsecond.
Remove the "hub-" prefix here.
> > > +Example:
> > > +
> > > + usb_hub1 {
> > > + compatible = "generic-onboard-hub";
> > > + clocks = <&clks IMX6QDL_CLK_CKO>;
> > > + clock-names = "external_clk";
> > > + hub-reset-gpios = <&gpio7 12 0>;
> > > + hub-reset-active-high;
> >
> > I think you could drop the 'hub-reset-active-high' property and do
> > like this instead:
> >
> > hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_HIGH>;
> >
> > or
> >
> > hub-reset-gpios = <&gpio7 12 GPIO_ACTIVE_LOW>;
>
> Yes, you are right, would you have a test to see if can work for
> udoo board?
Yes, it should be done like this.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] stmmac: create of compatible mdio bus for stmacc driver
On Tuesday 08 December 2015 13:12:59 Phil Reid wrote:
> @@ -201,6 +201,25 @@ int stmmac_mdio_register(struct net_device *ndev)
> struct stmmac_mdio_bus_data *mdio_bus_data = priv->plat->mdio_bus_data;
> int addr, found;
>
> +#ifdef CONFIG_OF
> + struct device_node *mdio_node = NULL;
> + struct device_node *child_node = NULL;
> +
> + for_each_child_of_node(priv->device->of_node, child_node) {
> + if (of_device_is_compatible(child_node, "snps,dwmac-mdio")) {
> + mdio_node = child_node;
> + break;
> + }
> + }
Can you use "if (IS_ENABLED(CONFIG_OF))" here instead of a preprocessor "#if"?
> @@ -231,7 +250,11 @@ int stmmac_mdio_register(struct net_device *ndev)
> new_bus->irq = irqlist;
> new_bus->phy_mask = mdio_bus_data->phy_mask;
> new_bus->parent = priv->device;
> +#ifdef CONFIG_OF
> + err = of_mdiobus_register(new_bus, mdio_node);
> +#else
> err = mdiobus_register(new_bus);
> +#endif
This looks like it should be done in the header file. Can you make
a separate patch that changes the header file declaring of_mdiobus_register
to make it a static inline function calling mdiobus_register() if CONFIG_OF
is disabled?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] stmmac: socfpga: Provide dt node to config ptp clk source.
On Monday 07 December 2015 21:34:29 Phil Reid wrote: > On 7/12/2015 5:05 PM, Arnd Bergmann wrote: > > On Monday 07 December 2015 09:38:44 Phil Reid wrote: > >> Signed-off-by: Phil Reid > >> --- > >> Documentation/devicetree/bindings/net/socfpga-dwmac.txt | 2 ++ > >> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 9 + > >> 2 files changed, 11 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > >> b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > >> index 3a9d679..72d82d6 100644 > >> --- a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > >> +++ b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > >> @@ -11,6 +11,8 @@ Required properties: > >>designware version numbers documented in stmmac.txt > >>- altr,sysmgr-syscon : Should be the phandle to the system manager node > >> that > >> encompasses the glue register, the register offset, and the register > >> shift. > >> + - altr,f2h_ptp_ref_clk use f2h_ptp_ref_clk instead of default eosc1 clock > >> + for ptp ref clk. This affects all emacs as the clock is common. > >> > > > > Is this feature specific to the Altera glue logic, or would it be possible > > to do the same thing on another dwmac implementation? > > > I think it is specific to Altera's glue logic. It selects either a clock > connected > directly to the ARM HPS core or a clock routed from Altera FPGA fabric. > Control register is in the altera sysmgr. > > Ok, makes sense. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] stmmac: Add ptp debugfs entry.
On Monday 07 December 2015 21:37:52 Phil Reid wrote: > On 7/12/2015 5:03 PM, Arnd Bergmann wrote: > > On Monday 07 December 2015 09:38:43 Phil Reid wrote: > >> This adds a debugfs entry to view the current status of the ptp > >> registers. > >> > >> Signed-off-by: Phil Reid > >> > > > > Your description should explain what this is good for. Why do you > > need to look at this through debugfs? > > > > Happy to drop this one. I found it helpful when debugging the ptp > behaviour. Allowing quick access to monitor the register updates > by the ptp driver. Is there an alternative method that is preferred? For tracing what the driver does, I'd add a trace event, which also lets you see when any updates happen. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
arRe: [PATCH net-next 2/2] net: hns: enet specisies a reference to dsaf (config and documents)
On Monday 07 December 2015 15:14:13 Yankejian wrote:
> On 2015/12/6 6:19, Arnd Bergmann wrote:
> > On Saturday 05 December 2015 14:10:56 yankejian wrote:
> >> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> >> b/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> >> index 80411b2..ecacfa4 100644
> >> --- a/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> >> +++ b/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> >> @@ -4,8 +4,6 @@ Required properties:
> >> - compatible: should be "hisilicon,hns-dsaf-v1" or
> >> "hisilicon,hns-dsaf-v2".
> >>"hisilicon,hns-dsaf-v1" is for hip05.
> >>"hisilicon,hns-dsaf-v2" is for Hi1610 and Hi1612.
> >> -- dsa-name: dsa fabric name who provide this interface.
> >> - should be "dsafX", X is the dsaf id.
> >> - mode: dsa fabric mode string. only support one of dsaf modes like these:
> >> "2port-64vf",
> >> "6port-16rss",
> >> @@ -26,9 +24,8 @@ Required properties:
> >>
> >> Example:
> >>
> >> -dsa: dsa@c700 {
> >> +dsaf0: dsa@c700 {
> >> compatible = "hisilicon,hns-dsaf-v1";
> >> - dsa_name = "dsaf0";
> >> mode = "6port-16rss";
> >> interrupt-parent = <&mbigen_dsa>;
> >> reg = <0x0 0xC000 0x0 0x42
> >> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hns-nic.txt
> >> b/Documentation/devicetree/bindings/net/hisilicon-hns-nic.txt
> >> index 41d19be..e6a9d1c 100644
> >> --- a/Documentation/devicetree/bindings/net/hisilicon-hns-nic.txt
> >> +++ b/Documentation/devicetree/bindings/net/hisilicon-hns-nic.txt
> >> @@ -4,8 +4,9 @@ Required properties:
> >> - compatible: "hisilicon,hns-nic-v1" or "hisilicon,hns-nic-v2".
> >>"hisilicon,hns-nic-v1" is for hip05.
> >>"hisilicon,hns-nic-v2" is for Hi1610 and Hi1612.
> >> -- ae-name: accelerator name who provides this interface,
> >> - is simply a name referring to the name of name in the accelerator node.
> >> +- ae-handle: accelerator engine handle for hns,
> >> + specifies a reference to the associating hardware driver node.
> >> + see Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> >> - port-id: is the index of port provided by DSAF (the accelerator). DSAF
> >> can
> >>connect to 8 PHYs. Port 0 to 1 are both used for adminstration purpose.
> >> They
> >>are called debug ports.
> >> @@ -41,7 +42,7 @@ Example:
> >>
> >>
> > This looks like an incompatible change, as you add and remove
> > required properties. Is there a way to support both the old and
> > the new style?
> >
> > Arnd
> >
> > .
>
> Hi Arnd,
> Thanks for your suggestions. it must be set the same strings in dsaf node
> and every enet node before.
> it seems inappropriate. as Rob Herring 's suggestions, that
> would solve associating
> enet with a particular dsaf. so we discus the solution with Yisen Zhuang
> .
> we decide to use the new way instead of the old one.
I agree the new form looks better than the original way, but I'm worried
about the migration path. You don't explain in the patch description
how you want to ensure that nothing breaks for existing systems.
We generally try to avoid doing incompatible changes altogether and
prefer to keep backwards compatibility, unless we can prove that no
other systems exist that would get impacted by the change.
Are you sure that nobody ships a DTB file for this hardware with their
firmware that would now require an incompatible update which in turn
breaks old kernels?
Are you sure that there is no hardware using the same dsa hardware
with out-of-tree dts files that need to make the same change but might
not be aware of the change?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/9] clk: hi3519: add dt-binding document and header file
On Monday 07 December 2015 16:01:03 xuejiancheng wrote: > On 2015/12/4 18:56, Arnd Bergmann wrote: > > On Friday 04 December 2015 11:21:28 xuejiancheng wrote: > >> Hi Arnd, > >> > >> On 2015/12/3 17:44, Arnd Bergmann wrote: > >>> On Thursday 03 December 2015 10:39:24 Jiancheng Xue wrote: > >>>> +#ifndef __DTS_HI3519_CLOCK_H > >>>> +#define __DTS_HI3519_CLOCK_H > >>> > >>> Please try to avoid adding headers like this if you can at all. > >>> > >>> I might ask you to merge the header file in one merge window > >>> otherwise and submit the platform code one kernel later, as they > >>> tendn to cause us needless dependencies otherwise. > >>> > >> > >> Sorry. In v1, Rob suggested putting binding doc and header files in > >> a separate patch. The clock driver indeed depends on the header. > >> > >> I will put the header and the clock driver in a patch, and keep the > >> binding doc in another patch. > > > > Having split patches is better, I was really commenting on the fact > > that ideally you would not have a header file at all. If we merge > > the header through arm-soc, then you won't be able to merge the > > clk driver easily, and if you merge the header through the clk > > maintainer, I'm can't take your dts files. > > Thank you for your comments. Because the clocks in the crg module have > different types and random layouts. If this header file is removed, > the clock driver and the dts files will get very complicated. > > Could you help me acknowledge it if I put the header file and clock driver > in a patch? > > Could you give me some suggestions If I want to keep this header file? If this is another clock controller that has a random register layout, then adding the header file is the least problematic solution indeed. > >>> Where do those numbers come from? They are not consecutive, so it sounds > >>> like they are directly from the data sheet and won't be needed in the > >>> driver. > >>> If that's true, just use the numbers directly, as you do for everything > >>> else. > >> > >> The numbers are defined by myself, not directly from the data sheet. Some > >> numbers > >> are reserved for device nodes which will be added later. So they are not > >> consecutive now. > > > > I don't understand. How do you decide which numbers to reserve? If the > > numbers are completely arbitrary and you have no idea what other clocks > > there are, you can simply have consecutive numbers here and do the driver > > accordingly. > > The clocks are divided into several groups according to their types. The > clocks in > a group are expected to have consecutive numbers. So some numbers are > reserved for > every group in this file, just like in some existing headers. Other clocks > will be > added when other peripherals drivers are submitted. They will use the reserve > numbers > and be inserted into existing groups. > > Of course it is not required to reserve numbers for later added clocks. Are you able to enumerate all the clocks then? If all clocks that are supported by this clock controllers have names in the data sheet, I would prefer to just list them all now rather than only enter the ones we already need, to avoid having future merge conflicts. Then we just need to add code to support those clocks when we need them, but that can be done in parallel to adding the DT nodes and the driver, rather than strongly serializing the patch flow on the header file patches. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] dt-bindings: add Marvell PMU documentation
On Sunday 06 December 2015 23:52:21 Russell King wrote: > Add the required DT binding documentation for the Marvell PMU driver. > > Acked-by: Rob Herring > Signed-off-by: Russell King > --- > Who takes these patches? This never got merged when the PMU driver > itself was merged. I'd say it should go along with the dts changes for mvebu, and we'll merge it through arm-soc. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] stmmac: socfpga: Provide dt node to config ptp clk source.
On Monday 07 December 2015 09:38:44 Phil Reid wrote: > Signed-off-by: Phil Reid > --- > Documentation/devicetree/bindings/net/socfpga-dwmac.txt | 2 ++ > drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 9 + > 2 files changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > index 3a9d679..72d82d6 100644 > --- a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > +++ b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt > @@ -11,6 +11,8 @@ Required properties: > designware version numbers documented in stmmac.txt > - altr,sysmgr-syscon : Should be the phandle to the system manager node that > encompasses the glue register, the register offset, and the register > shift. > + - altr,f2h_ptp_ref_clk use f2h_ptp_ref_clk instead of default eosc1 clock > + for ptp ref clk. This affects all emacs as the clock is common. > Is this feature specific to the Altera glue logic, or would it be possible to do the same thing on another dwmac implementation? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] stmmac: Add ptp debugfs entry.
On Monday 07 December 2015 09:38:43 Phil Reid wrote: > This adds a debugfs entry to view the current status of the ptp > registers. > > Signed-off-by: Phil Reid > Your description should explain what this is good for. Why do you need to look at this through debugfs? Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
