Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
From: Rob Herring @ 2020-05-29 17:34 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: Nicolas Saenz Julienne,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
	Christoph Hellwig, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	Frank Rowand, Greg Kroah-Hartman, Marek Szyprowski, Robin Murphy,
	Alan Stern, Oliver Neukum, Rafael J. Wysocki, Andy Shevchenko,
	Wolfram Sang, Corey Minyard, Srinivas Kandagatla,
	Suzuki K Poulose, Saravana Kannan, Heikki Krogerus, Dan Williams,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list,
	open list:USB SUBSYSTEM, open list:DMA MAPPING HELPERS
In-Reply-To: <CA+-6iNyOKvY-xNfXqDRa5_nJVJuqGKA-oe-ejNuJHUBt6ORu0A@mail.gmail.com>

On Wed, May 27, 2020 at 9:43 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
>
> Hi Nicolas,
>
> On Wed, May 27, 2020 at 11:00 AM Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> >
> > Hi Jim,
> > one thing comes to mind, there is a small test suite in drivers/of/unittest.c
> > (specifically of_unittest_pci_dma_ranges()) you could extend it to include your
> > use cases.
> Sure, will check out.
> >
> > On Tue, 2020-05-26 at 15:12 -0400, Jim Quinlan wrote:
> > > The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> > > the use of multiple pfn offsets between cpu addrs and dma addrs.  It is
> > > similar to 'dma_pfn_offset' except that the offset chosen depends on the
> > > cpu or dma address involved.
> > >
> > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > > ---
> > >  drivers/of/address.c        | 65 +++++++++++++++++++++++++++++++++++--
> > >  drivers/usb/core/message.c  |  3 ++
> > >  drivers/usb/core/usb.c      |  3 ++
> > >  include/linux/device.h      | 10 +++++-
> > >  include/linux/dma-direct.h  | 10 ++++--
> > >  include/linux/dma-mapping.h | 46 ++++++++++++++++++++++++++
> > >  kernel/dma/Kconfig          | 13 ++++++++
> > >  7 files changed, 144 insertions(+), 6 deletions(-)
> > >
> >
> > [...]
> >
> > > @@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
> > > device_node *np, u64 *dma_addr,
> > >               pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> > >                        range.bus_addr, range.cpu_addr, range.size);
> > >
> > > +             num_ranges++;
> > >               if (dma_offset && range.cpu_addr - range.bus_addr != dma_offset)
> > > {
> > > -                     pr_warn("Can't handle multiple dma-ranges with different
> > > offsets on node(%pOF)\n", node);
> > > -                     /* Don't error out as we'd break some existing DTs */
> > > -                     continue;
> > > +                     if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
> > > +                             pr_warn("Can't handle multiple dma-ranges with
> > > different offsets on node(%pOF)\n", node);
> > > +                             pr_warn("Perhaps set DMA_PFN_OFFSET_MAP=y?\n");
> > > +                             /*
> > > +                              * Don't error out as we'd break some existing
> > > +                              * DTs that are using configs w/o
> > > +                              * CONFIG_DMA_PFN_OFFSET_MAP set.
> > > +                              */
> > > +                             continue;
> >
> > dev->bus_dma_limit is set in of_dma_configure(), this function's caller, based
> > on dma_start's value (set after this continue). So you'd be effectively setting
> > the dev->bus_dma_limit to whatever we get from the first dma-range.
> I'm not seeing that at all.  On the  evaluation of each dma-range,
> dma_start and dma_end are re-evaluated to be the lowest and highest
> bus values of the  dma-ranges seen so far.  After all dma-ranges are
> examined,  dev->bus_dma_limit being set to the highest.  In fact, the
> current code -- ie before my commits -- already does this for multiple
> dma-ranges as long as the cpu-bus offset is the same in the
> dma-ranges.
> >
> > This can be troublesome depending on how the dma-ranges are setup, for example
> > if the first dma-range doesn't include the CMA area, in arm64 generally set as
> > high as possible in ZONE_DMA32, that would render it useless for
> > dma/{direct/swiotlb}. Again depending on the bus_dma_limit value, if smaller
> > than ZONE_DMA you'd be unable to allocate any DMA memory.
> >
> > IMO, a solution to this calls for a revamp of dma-direct's dma_capable(): match
> > the target DMA memory area with each dma-range we have to see if it fits.
> >
> > > +                     }
> > > +                     dma_multi_pfn_offset = true;
> > >               }
> > >               dma_offset = range.cpu_addr - range.bus_addr;
> > >
> > > @@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
> > > device_node *np, u64 *dma_addr,
> > >                       dma_end = range.bus_addr + range.size;
> > >       }
> > >
> > > +     if (dma_multi_pfn_offset) {
> > > +             dma_offset = 0;
> > > +             ret = attach_dma_pfn_offset_map(dev, node, num_ranges);
> > > +             if (ret)
> > > +                     return ret;
> > > +     }
> > > +
> > >       if (dma_start >= dma_end) {
> > >               ret = -EINVAL;
> > >               pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
> > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > > index 6197938dcc2d..aaa3e58f5eb4 100644
> > > --- a/drivers/usb/core/message.c
> > > +++ b/drivers/usb/core/message.c
> > > @@ -1960,6 +1960,9 @@ int usb_set_configuration(struct usb_device *dev, int
> > > configuration)
> > >                */
> > >               intf->dev.dma_mask = dev->dev.dma_mask;
> > >               intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
> > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > +             intf->dev.dma_pfn_offset_map = dev->dev.dma_pfn_offset_map;
> > > +#endif
> >
> > Thanks for looking at this, that said, I see more instances of drivers changing
> > dma_pfn_offset outside of the core code. Why not doing this there too?
> >
> > Also, are we 100% sure that dev->dev.dma_pfn_offset isn't going to be freed
> > before we're done using intf->dev? Maybe it's safer to copy the ranges?
> >
> > >               INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
> > >               intf->minor = -1;
> > >               device_initialize(&intf->dev);
> > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > > index f16c26dc079d..d2ed4d90e56e 100644
> > > --- a/drivers/usb/core/usb.c
> > > +++ b/drivers/usb/core/usb.c
> > > @@ -612,6 +612,9 @@ struct usb_device *usb_alloc_dev(struct usb_device
> > > *parent,
> > >        */
> > >       dev->dev.dma_mask = bus->sysdev->dma_mask;
> > >       dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;
> > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > +     dev->dev.dma_pfn_offset_map = bus->sysdev->dma_pfn_offset_map;
> > > +#endif
> > >       set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
> > >       dev->state = USB_STATE_ATTACHED;
> > >       dev->lpm_disable_count = 1;
> > > diff --git a/include/linux/device.h b/include/linux/device.h
> > > index ac8e37cd716a..67a240ad4fc5 100644
> > > --- a/include/linux/device.h
> > > +++ b/include/linux/device.h
> > > @@ -493,6 +493,8 @@ struct dev_links_info {
> > >   * @bus_dma_limit: Limit of an upstream bridge or bus which imposes a smaller
> > >   *           DMA limit than the device itself supports.
> > >   * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> > > + * @dma_pfn_offset_map:      Like dma_pfn_offset but used when there are
> > > multiple
> > > + *           pfn offsets for multiple dma-ranges.
> > >   * @dma_parms:       A low level driver may set these to teach IOMMU code
> > > about
> > >   *           segment limitations.
> > >   * @dma_pools:       Dma pools (if dma'ble device).
> > > @@ -578,7 +580,13 @@ struct device {
> > >                                            allocations such descriptors. */
> > >       u64             bus_dma_limit;  /* upstream dma constraint */
> > >       unsigned long   dma_pfn_offset;
> > > -
> > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > +     const struct dma_pfn_offset_region *dma_pfn_offset_map;
> > > +                                     /* Like dma_pfn_offset, but for
> > > +                                      * the unlikely case of multiple
> > > +                                      * offsets. If non-null, dma_pfn_offset
> > > +                                      * will be set to 0. */
> > > +#endif
> >
> > I'm still sad this doesn't fully replace dma_pfn_offset & bus_dma_limit. I feel
> > the extra logic involved in incorporating this as default isn't going to be
> > noticeable as far as performance is concerned to single dma-range users, and
> > it'd make for a nicer DMA code. Also you'd force everyone to test their changes
> > on the multi dma-ranges code path, as opposed to having this disabled 99.9% of
> > the time (hence broken every so often).
> Good point.

+1

> > Note that I sympathize with the amount of work involved on improving that, so
> > better wait to hear what more knowledgeable people have to say about this :)
> Yes, I agree.  I want to avoid coding and testing one solution only to
> have a different reviewer NAK it.

It's a pretty safe bet that everyone will prefer one code path over 2.

Rob

^ permalink raw reply

* Re: [PATCH v6 00/16] spi: dw: Add generic DW DMA controller support
From: Serge Semin @ 2020-05-29 17:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Serge Semin, Ekaterina Skachko, Feng Tang, devicetree,
	Thomas Bogendoerfer, Georgy Vlasov, Pavel Parkhomenko,
	Alexey Kolotnikov, linux-spi, linux-kernel, Vadim Vlasov,
	Alexey Malahov, linux-mips, Andy Shevchenko, Rob Herring,
	Ramil Zaripov, Arnd Bergmann, Maxim Kaurkin
In-Reply-To: <20200529173325.GR4610@sirena.org.uk>

On Fri, May 29, 2020 at 06:33:25PM +0100, Mark Brown wrote:
> On Fri, May 29, 2020 at 08:26:42PM +0300, Serge Semin wrote:
> 
> > You must have missed the patch 16:
> > 0e8332aaf059 dt-bindings: spi: Convert DW SPI binding to DT schema
> > As you can see it has been acked by Rob. So you can also merge it into your
> > repo. Though It has to be rebased first due to the Dinh Nguyen patches
> > recently merged in. Do you want me to do the rebasing?
> 
> Please rebase.  TBH I'd not noticed Rob's review so I just left it
> waiting for that, there's such a huge backlog there it didn't occur to
> me that it might've been reviewed.

Ok. I'll do the rebasing shortly. (also have to add the optional reset properties
into the bindings)

-Sergey

^ permalink raw reply

* Re: [PATCH] dt-bindings: leds: fix macro names for pca955x
From: Rob Herring @ 2020-05-29 17:27 UTC (permalink / raw)
  To: Flavio Suligoi
  Cc: linux-kernel, Dan Murphy, devicetree, Pavel Machek,
	Jacek Anaszewski, Rob Herring, linux-leds
In-Reply-To: <20200526092052.24172-1-f.suligoi@asem.it>

On Tue, 26 May 2020 11:20:52 +0200, Flavio Suligoi wrote:
> The documentation reports the wrong macro names
> related to the pca9532 instead of the pca955x
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  Documentation/devicetree/bindings/leds/leds-pca955x.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v6 00/16] spi: dw: Add generic DW DMA controller support
From: Serge Semin @ 2020-05-29 17:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: Serge Semin, Ekaterina Skachko, Feng Tang, devicetree,
	Thomas Bogendoerfer, Georgy Vlasov, Pavel Parkhomenko,
	Alexey Kolotnikov, linux-spi, linux-kernel, Vadim Vlasov,
	Alexey Malahov, linux-mips, Andy Shevchenko, Rob Herring,
	Ramil Zaripov, Arnd Bergmann, Maxim Kaurkin
In-Reply-To: <159077271266.17043.13820488074564153429.b4-ty@kernel.org>

Mark

On Fri, May 29, 2020 at 06:18:32PM +0100, Mark Brown wrote:
> On Fri, 29 May 2020 16:11:49 +0300, Serge Semin wrote:
> > Baikal-T1 SoC provides a DW DMA controller to perform low-speed peripherals
> > Mem-to-Dev and Dev-to-Mem transaction. This is also applicable to the DW
> > APB SSI devices embedded into the SoC. Currently the DMA-based transfers
> > are supported by the DW APB SPI driver only as a middle layer code for
> > Intel MID/Elkhart PCI devices. Seeing the same code can be used for normal
> > platform DMAC device we introduced a set of patches to fix it within this
> > series.
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> 
> Thanks!
> 
> [01/15] spi: dw: Set xfer effective_speed_hz
>         commit: de4c2875a5ff2c886df60f2086c6affca83f890a
> [02/15] spi: dw: Return any value retrieved from the dma_transfer callback
>         commit: f0410bbf7d0fb80149e3b17d11d31f5b5197873e
> [03/15] spi: dw: Locally wait for the DMA transfers completion
>         commit: bdbdf0f06337d3661b64c0288c291cb06624065e
> [04/15] spi: dw: Add SPI Tx-done wait method to DMA-based transfer
>         commit: 1ade2d8a72f9240825f6be050f0d49c840f7daeb
> [05/15] spi: dw: Add SPI Rx-done wait method to DMA-based transfer
>         commit: 33726eff3d98e643f7d7a0940f4024844b430c82
> [06/15] spi: dw: Parameterize the DMA Rx/Tx burst length
>         commit: c534df9d6225314d1403e4330a22d68c35e0eb55
> [07/15] spi: dw: Use DMA max burst to set the request thresholds
>         commit: 0b2b66514fc9971b3a6002ba038d74f77705fd34
> [08/15] spi: dw: Fix Rx-only DMA transfers
>         commit: 46164fde6b7890e7a3982d54549947c8394c0192
> [09/15] spi: dw: Add core suffix to the DW APB SSI core source file
>         commit: 77ccff803d27279ccc100dc906c6f456c8fa515c
> [10/15] spi: dw: Move Non-DMA code to the DW PCIe-SPI driver
>         commit: 6c710c0cb6725bdbe647b958756685aed0295936
> [11/15] spi: dw: Remove DW DMA code dependency from DW_DMAC_PCI
>         commit: 06cfadb8c51b05c6b91c2d43e0fe72b3d643dced
> [12/15] spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core
>         commit: ecb3a67edfd353837dc23b538fb250d1dfd88e7b
> [13/15] spi: dw: Cleanup generic DW DMA code namings
>         commit: 57784411728ff4d72ae051fdbba1e54fcb1f8d6f
> [14/15] spi: dw: Add DMA support to the DW SPI MMIO driver
>         commit: 0fdad596d46b28d5c3e39d1897c5e3878b64d9a2
> [15/15] spi: dw: Use regset32 DebugFS method to create regdump file
>         commit: 8378449d1f79add31be77e77fc7df9f639878e9c

You must have missed the patch 16:
0e8332aaf059 dt-bindings: spi: Convert DW SPI binding to DT schema
As you can see it has been acked by Rob. So you can also merge it into your
repo. Though It has to be rebased first due to the Dinh Nguyen patches
recently merged in. Do you want me to do the rebasing?

-Sergey

> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark

^ permalink raw reply

* Re: [PATCH 1/1] dt-bindings: MIPS: Document Ingenic SoCs binding.
From: Rob Herring @ 2020-05-29 17:39 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: 周琰杰, linux-mips, linux-kernel, devicetree,
	tsbogend, hns, paul, dongsheng.qiu, aric.pzqi, sernia.zhou,
	zhenwenjin
In-Reply-To: <H9DYAQ.4YAB8VVZPLZO@crapouillou.net>

On Tue, May 26, 2020 at 09:10:29PM +0200, Paul Cercueil wrote:
> Hi Zhou,
> 
> Le mer. 27 mai 2020 à 1:07, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> a écrit :
> > Document the available properties for the SoC root node and the
> > CPU nodes of the devicetree for the Ingenic XBurst SoCs.
> > 
> > Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> > Tested-by: Paul Boddie <paul@boddie.org.uk>
> > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> > ---
> >  .../bindings/mips/ingenic/ingenic,cpu.yaml         | 57
> > ++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> > b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> > new file mode 100644
> > index 000000000000..afb02071a756
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Bindings for Ingenic XBurst family CPUs
> > +
> > +maintainers:
> > +  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> > +
> > +description:
> > +  Ingenic XBurst family CPUs shall have the following properties.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +
> > +      - description: Ingenic XBurst®1 CPU Cores
> > +        items:
> 
> Strip the 'items', put the enum directly.
> 
> > +          enum:
> > +            - ingenic,xburst-mxu1.0
> > +            - ingenic,xburst-fpu1.0-mxu1.1
> > +            - ingenic,xburst-fpu2.0-mxu2.0
> > +
> > +      - description: Ingenic XBurst®2 CPU Cores
> > +        items:
> 
> Same here.
> 
> > +          enum:
> > +            - ingenic,xburst2-fpu2.1-mxu2.1-smt
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - device_type
> > +  - compatible
> > +  - reg
> 
> device_type is not in the list of your properties.

It doesn't have to be. There's already a schema for it in dt-schema. 
It's not always required there, so requiring here is fine.

It's an oddity of json-schema, but what's listed in required doesn't 
have to be in 'properties'.

Rob

^ permalink raw reply

* Re: [PATCH v6 00/16] spi: dw: Add generic DW DMA controller support
From: Andy Shevchenko @ 2020-05-29 17:43 UTC (permalink / raw)
  To: Serge Semin
  Cc: Mark Brown, Serge Semin, Ekaterina Skachko, Feng Tang, devicetree,
	Thomas Bogendoerfer, Georgy Vlasov, Pavel Parkhomenko,
	Alexey Kolotnikov, linux-spi, linux-kernel, Vadim Vlasov,
	Alexey Malahov, linux-mips, Rob Herring, Ramil Zaripov,
	Arnd Bergmann, Maxim Kaurkin
In-Reply-To: <20200529172642.hcnvyzv2ocizsvpy@mobilestation>

On Fri, May 29, 2020 at 08:26:42PM +0300, Serge Semin wrote:
> On Fri, May 29, 2020 at 06:18:32PM +0100, Mark Brown wrote:
> > On Fri, 29 May 2020 16:11:49 +0300, Serge Semin wrote:
> > > Baikal-T1 SoC provides a DW DMA controller to perform low-speed peripherals
> > > Mem-to-Dev and Dev-to-Mem transaction. This is also applicable to the DW
> > > APB SSI devices embedded into the SoC. Currently the DMA-based transfers
> > > are supported by the DW APB SPI driver only as a middle layer code for
> > > Intel MID/Elkhart PCI devices. Seeing the same code can be used for normal
> > > platform DMAC device we introduced a set of patches to fix it within this
> > > series.

> As you can see it has been acked by Rob. So you can also merge it into your
> repo. Though It has to be rebased first due to the Dinh Nguyen patches
> recently merged in. Do you want me to do the rebasing?

I guess now you need to rebase it, because as I see the Dinh's patches are in
the tree as well as yours.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v2 03/14] dt-bindings: PCI: Add bindings for more Brcmstb chips
From: Rob Herring @ 2020-05-29 17:46 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
	bcm-kernel-feedback-list, Florian Fainelli, Bjorn Helgaas,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <20200526191303.1492-4-james.quinlan@broadcom.com>

On Tue, May 26, 2020 at 03:12:42PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
> 
> - Add compatible strings for three more Broadcom STB chips: 7278, 7216,
>   7211 (STB version of RPi4).
> - add new property 'brcm,scb-sizes'
> - add new property 'resets'
> - add new property 'reset-names'
> - allow 'ranges' and 'dma-ranges' to have more than one item and update
>   the example to show this.
> 
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> ---
>  .../bindings/pci/brcm,stb-pcie.yaml           | 40 +++++++++++++++++--
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> index 8680a0f86c5a..66a7df45983d 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> @@ -14,7 +14,13 @@ allOf:
>  
>  properties:
>    compatible:
> -    const: brcm,bcm2711-pcie # The Raspberry Pi 4
> +    items:
> +      - enum:

Don't need items here. Just change the const to enum.

> +          - brcm,bcm2711-pcie # The Raspberry Pi 4
> +          - brcm,bcm7211-pcie # Broadcom STB version of RPi4
> +          - brcm,bcm7278-pcie # Broadcom 7278 Arm
> +          - brcm,bcm7216-pcie # Broadcom 7216 Arm
> +          - brcm,bcm7445-pcie # Broadcom 7445 Arm
>  
>    reg:
>      maxItems: 1
> @@ -34,10 +40,12 @@ properties:
>        - const: msi
>  
>    ranges:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 4
>  
>    dma-ranges:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 6
>  
>    clocks:
>      maxItems: 1
> @@ -58,8 +66,30 @@ properties:
>  
>    aspm-no-l0s: true
>  
> +  resets:
> +    description: for "brcm,bcm7216-pcie", must be a valid reset
> +      phandle pointing to the RESCAL reset controller provider node.
> +    $ref: "/schemas/types.yaml#/definitions/phandle"
> +
> +  reset-names:
> +    items:
> +      - const: rescal

These are going to need to be an if/then schema if they only apply to 
certain compatible(s).

> +
> +  brcm,scb-sizes:
> +    description: (u32, u32) tuple giving the 64bit PCIe memory
> +      viewport size of a memory controller.  There may be up to
> +      three controllers, and each size must be a power of two
> +      with a size greater or equal to the amount of memory the
> +      controller supports.

This sounds like what dma-ranges should express?

If not, we do have 64-bit size if that what you need.

> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> +      - items:
> +          minItems: 2
> +          maxItems: 6
> +
>  required:
>    - reg
> +  - ranges
>    - dma-ranges
>    - "#interrupt-cells"
>    - interrupts
> @@ -93,7 +123,9 @@ examples:
>                      msi-parent = <&pcie0>;
>                      msi-controller;
>                      ranges = <0x02000000 0x0 0xf8000000 0x6 0x00000000 0x0 0x04000000>;
> -                    dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>;
> +                    dma-ranges = <0x42000000 0x1 0x00000000 0x0 0x40000000 0x0 0x80000000>,
> +                                 <0x42000000 0x1 0x80000000 0x3 0x00000000 0x0 0x80000000>;
>                      brcm,enable-ssc;
> +                    brcm,scb-sizes = <0x0 0x80000000 0x0 0x80000000>;
>              };
>      };
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH v6 00/16] spi: dw: Add generic DW DMA controller support
From: Serge Semin @ 2020-05-29 17:47 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Mark Brown, Ekaterina Skachko, Feng Tang, devicetree,
	Thomas Bogendoerfer, Georgy Vlasov, Pavel Parkhomenko,
	Alexey Kolotnikov, linux-spi, linux-kernel, Vadim Vlasov,
	Alexey Malahov, linux-mips, Rob Herring, Ramil Zaripov,
	Arnd Bergmann, Maxim Kaurkin
In-Reply-To: <20200529174312.GU1634618@smile.fi.intel.com>

On Fri, May 29, 2020 at 08:43:12PM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 08:26:42PM +0300, Serge Semin wrote:
> > On Fri, May 29, 2020 at 06:18:32PM +0100, Mark Brown wrote:
> > > On Fri, 29 May 2020 16:11:49 +0300, Serge Semin wrote:
> > > > Baikal-T1 SoC provides a DW DMA controller to perform low-speed peripherals
> > > > Mem-to-Dev and Dev-to-Mem transaction. This is also applicable to the DW
> > > > APB SSI devices embedded into the SoC. Currently the DMA-based transfers
> > > > are supported by the DW APB SPI driver only as a middle layer code for
> > > > Intel MID/Elkhart PCI devices. Seeing the same code can be used for normal
> > > > platform DMAC device we introduced a set of patches to fix it within this
> > > > series.
> 
> > As you can see it has been acked by Rob. So you can also merge it into your
> > repo. Though It has to be rebased first due to the Dinh Nguyen patches
> > recently merged in. Do you want me to do the rebasing?
> 
> I guess now you need to rebase it, because as I see the Dinh's patches are in
> the tree as well as yours.

Right. That's what I am doing at the moment.)

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH v2 00/14] PCI: brcmstb: enable PCIe for STB chips
From: Rob Herring @ 2020-05-29 17:48 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: linux-pci, Christoph Hellwig, Nicolas Saenz Julienne,
	bcm-kernel-feedback-list, Alan Stern, Andy Shevchenko,
	Corey Minyard, Dan Williams,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE,
	Greg Kroah-Hartman, Heikki Krogerus,
	open list:DMA MAPPING HELPERS, Julien Grall,
	moderated list:ARM PORT,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:USB SUBSYSTEM, Mark Brown, Oliver Neukum,
	Rafael J. Wysocki, Robin Murphy, Saravana Kannan,
	Stefano Stabellini, Suzuki K Poulose, Ulf Hansson, Wolfram Sang
In-Reply-To: <20200526191303.1492-1-james.quinlan@broadcom.com>

On Tue, May 26, 2020 at 03:12:39PM -0400, Jim Quinlan wrote:
> v2:
> Commit: "device core: Add ability to handle multiple dma offsets"
>   o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
>   o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
>   o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
>   o dev->dma_pfn_map => dev->dma_pfn_offset_map
>   o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
>   o In device.h: s/const void */const struct dma_pfn_offset_region */
>   o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
>     guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
>   o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
>     dev->dma_pfn_offset_map is copied as well.
>   o Merged two of the DMA commits into one (Christoph).
> 
> Commit "arm: dma-mapping: Invoke dma offset func if needed":
>   o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
> 
> Other commits' changes:
>   o Removed need for carrying of_id var in priv (Nicolas)
>   o Commit message rewordings (Bjorn)
>   o Commit log messages filled to 75 chars (Bjorn)
>   o devm_reset_control_get_shared())
>     => devm_reset_control_get_optional_shared (Philipp)
>   o Add call to reset_control_assert() in PCIe remove routines (Philipp)
> 
> v1:
> This patchset expands the usefulness of the Broadcom Settop Box PCIe
> controller by building upon the PCIe driver used currently by the
> Raspbery Pi.  Other forms of this patchset were submitted by me years
> ago and not accepted; the major sticking point was the code required
> for the DMA remapping needed for the PCIe driver to work [1].
> 
> There have been many changes to the DMA and OF subsystems since that
> time, making a cleaner and less intrusive patchset possible.  This
> patchset implements a generalization of "dev->dma_pfn_offset", except
> that instead of a single scalar offset it provides for multiple
> offsets via a function which depends upon the "dma-ranges" property of
> the PCIe host controller.  This is required for proper functionality
> of the BrcmSTB PCIe controller and possibly some other devices.

If you can enable the h/w support without the multiple offset support, 
then I'd split up this series. The latter part might take a bit more 
time.

Rob

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: ASoC: renesas,rsnd: Add r8a7742 support
From: Rob Herring @ 2020-05-29 17:50 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: alsa-devel, Prabhakar, Rob Herring, Magnus Damm, Mark Brown,
	linux-kernel, linux-renesas-soc, Liam Girdwood,
	Geert Uytterhoeven, devicetree
In-Reply-To: <1590526904-13855-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>

On Tue, 26 May 2020 22:01:43 +0100, Lad Prabhakar wrote:
> Document RZ/G1H (R8A7742) SoC bindings.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/sound/renesas,rsnd.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 09/14] device core: Add ability to handle multiple dma offsets
From: Jim Quinlan @ 2020-05-29 17:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: Nicolas Saenz Julienne,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
	Christoph Hellwig, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
	Frank Rowand, Greg Kroah-Hartman, Marek Szyprowski, Robin Murphy,
	Alan Stern, Oliver Neukum, Rafael J. Wysocki, Andy Shevchenko,
	Wolfram Sang, Corey Minyard, Srinivas Kandagatla,
	Suzuki K Poulose, Saravana Kannan, Heikki Krogerus, Dan Williams,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list,
	open list:USB SUBSYSTEM, open list:DMA MAPPING HELPERS
In-Reply-To: <CAL_JsqJsxxC6msUXBCa9naitMLfOcVZauk44gPJNGGe3iXRzsA@mail.gmail.com>

On Fri, May 29, 2020 at 1:35 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Wed, May 27, 2020 at 9:43 AM Jim Quinlan <james.quinlan@broadcom.com> wrote:
> >
> > Hi Nicolas,
> >
> > On Wed, May 27, 2020 at 11:00 AM Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > >
> > > Hi Jim,
> > > one thing comes to mind, there is a small test suite in drivers/of/unittest.c
> > > (specifically of_unittest_pci_dma_ranges()) you could extend it to include your
> > > use cases.
> > Sure, will check out.
> > >
> > > On Tue, 2020-05-26 at 15:12 -0400, Jim Quinlan wrote:
> > > > The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> > > > the use of multiple pfn offsets between cpu addrs and dma addrs.  It is
> > > > similar to 'dma_pfn_offset' except that the offset chosen depends on the
> > > > cpu or dma address involved.
> > > >
> > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> > > > ---
> > > >  drivers/of/address.c        | 65 +++++++++++++++++++++++++++++++++++--
> > > >  drivers/usb/core/message.c  |  3 ++
> > > >  drivers/usb/core/usb.c      |  3 ++
> > > >  include/linux/device.h      | 10 +++++-
> > > >  include/linux/dma-direct.h  | 10 ++++--
> > > >  include/linux/dma-mapping.h | 46 ++++++++++++++++++++++++++
> > > >  kernel/dma/Kconfig          | 13 ++++++++
> > > >  7 files changed, 144 insertions(+), 6 deletions(-)
> > > >
> > >
> > > [...]
> > >
> > > > @@ -977,10 +1020,19 @@ int of_dma_get_range(struct device *dev, struct
> > > > device_node *np, u64 *dma_addr,
> > > >               pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> > > >                        range.bus_addr, range.cpu_addr, range.size);
> > > >
> > > > +             num_ranges++;
> > > >               if (dma_offset && range.cpu_addr - range.bus_addr != dma_offset)
> > > > {
> > > > -                     pr_warn("Can't handle multiple dma-ranges with different
> > > > offsets on node(%pOF)\n", node);
> > > > -                     /* Don't error out as we'd break some existing DTs */
> > > > -                     continue;
> > > > +                     if (!IS_ENABLED(CONFIG_DMA_PFN_OFFSET_MAP)) {
> > > > +                             pr_warn("Can't handle multiple dma-ranges with
> > > > different offsets on node(%pOF)\n", node);
> > > > +                             pr_warn("Perhaps set DMA_PFN_OFFSET_MAP=y?\n");
> > > > +                             /*
> > > > +                              * Don't error out as we'd break some existing
> > > > +                              * DTs that are using configs w/o
> > > > +                              * CONFIG_DMA_PFN_OFFSET_MAP set.
> > > > +                              */
> > > > +                             continue;
> > >
> > > dev->bus_dma_limit is set in of_dma_configure(), this function's caller, based
> > > on dma_start's value (set after this continue). So you'd be effectively setting
> > > the dev->bus_dma_limit to whatever we get from the first dma-range.
> > I'm not seeing that at all.  On the  evaluation of each dma-range,
> > dma_start and dma_end are re-evaluated to be the lowest and highest
> > bus values of the  dma-ranges seen so far.  After all dma-ranges are
> > examined,  dev->bus_dma_limit being set to the highest.  In fact, the
> > current code -- ie before my commits -- already does this for multiple
> > dma-ranges as long as the cpu-bus offset is the same in the
> > dma-ranges.
> > >
> > > This can be troublesome depending on how the dma-ranges are setup, for example
> > > if the first dma-range doesn't include the CMA area, in arm64 generally set as
> > > high as possible in ZONE_DMA32, that would render it useless for
> > > dma/{direct/swiotlb}. Again depending on the bus_dma_limit value, if smaller
> > > than ZONE_DMA you'd be unable to allocate any DMA memory.
> > >
> > > IMO, a solution to this calls for a revamp of dma-direct's dma_capable(): match
> > > the target DMA memory area with each dma-range we have to see if it fits.
> > >
> > > > +                     }
> > > > +                     dma_multi_pfn_offset = true;
> > > >               }
> > > >               dma_offset = range.cpu_addr - range.bus_addr;
> > > >
> > > > @@ -991,6 +1043,13 @@ int of_dma_get_range(struct device *dev, struct
> > > > device_node *np, u64 *dma_addr,
> > > >                       dma_end = range.bus_addr + range.size;
> > > >       }
> > > >
> > > > +     if (dma_multi_pfn_offset) {
> > > > +             dma_offset = 0;
> > > > +             ret = attach_dma_pfn_offset_map(dev, node, num_ranges);
> > > > +             if (ret)
> > > > +                     return ret;
> > > > +     }
> > > > +
> > > >       if (dma_start >= dma_end) {
> > > >               ret = -EINVAL;
> > > >               pr_debug("Invalid DMA ranges configuration on node(%pOF)\n",
> > > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> > > > index 6197938dcc2d..aaa3e58f5eb4 100644
> > > > --- a/drivers/usb/core/message.c
> > > > +++ b/drivers/usb/core/message.c
> > > > @@ -1960,6 +1960,9 @@ int usb_set_configuration(struct usb_device *dev, int
> > > > configuration)
> > > >                */
> > > >               intf->dev.dma_mask = dev->dev.dma_mask;
> > > >               intf->dev.dma_pfn_offset = dev->dev.dma_pfn_offset;
> > > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > > +             intf->dev.dma_pfn_offset_map = dev->dev.dma_pfn_offset_map;
> > > > +#endif
> > >
> > > Thanks for looking at this, that said, I see more instances of drivers changing
> > > dma_pfn_offset outside of the core code. Why not doing this there too?
> > >
> > > Also, are we 100% sure that dev->dev.dma_pfn_offset isn't going to be freed
> > > before we're done using intf->dev? Maybe it's safer to copy the ranges?
> > >
> > > >               INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
> > > >               intf->minor = -1;
> > > >               device_initialize(&intf->dev);
> > > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > > > index f16c26dc079d..d2ed4d90e56e 100644
> > > > --- a/drivers/usb/core/usb.c
> > > > +++ b/drivers/usb/core/usb.c
> > > > @@ -612,6 +612,9 @@ struct usb_device *usb_alloc_dev(struct usb_device
> > > > *parent,
> > > >        */
> > > >       dev->dev.dma_mask = bus->sysdev->dma_mask;
> > > >       dev->dev.dma_pfn_offset = bus->sysdev->dma_pfn_offset;
> > > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > > +     dev->dev.dma_pfn_offset_map = bus->sysdev->dma_pfn_offset_map;
> > > > +#endif
> > > >       set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
> > > >       dev->state = USB_STATE_ATTACHED;
> > > >       dev->lpm_disable_count = 1;
> > > > diff --git a/include/linux/device.h b/include/linux/device.h
> > > > index ac8e37cd716a..67a240ad4fc5 100644
> > > > --- a/include/linux/device.h
> > > > +++ b/include/linux/device.h
> > > > @@ -493,6 +493,8 @@ struct dev_links_info {
> > > >   * @bus_dma_limit: Limit of an upstream bridge or bus which imposes a smaller
> > > >   *           DMA limit than the device itself supports.
> > > >   * @dma_pfn_offset: offset of DMA memory range relatively of RAM
> > > > + * @dma_pfn_offset_map:      Like dma_pfn_offset but used when there are
> > > > multiple
> > > > + *           pfn offsets for multiple dma-ranges.
> > > >   * @dma_parms:       A low level driver may set these to teach IOMMU code
> > > > about
> > > >   *           segment limitations.
> > > >   * @dma_pools:       Dma pools (if dma'ble device).
> > > > @@ -578,7 +580,13 @@ struct device {
> > > >                                            allocations such descriptors. */
> > > >       u64             bus_dma_limit;  /* upstream dma constraint */
> > > >       unsigned long   dma_pfn_offset;
> > > > -
> > > > +#ifdef CONFIG_DMA_PFN_OFFSET_MAP
> > > > +     const struct dma_pfn_offset_region *dma_pfn_offset_map;
> > > > +                                     /* Like dma_pfn_offset, but for
> > > > +                                      * the unlikely case of multiple
> > > > +                                      * offsets. If non-null, dma_pfn_offset
> > > > +                                      * will be set to 0. */
> > > > +#endif
> > >
> > > I'm still sad this doesn't fully replace dma_pfn_offset & bus_dma_limit. I feel
> > > the extra logic involved in incorporating this as default isn't going to be
> > > noticeable as far as performance is concerned to single dma-range users, and
> > > it'd make for a nicer DMA code. Also you'd force everyone to test their changes
> > > on the multi dma-ranges code path, as opposed to having this disabled 99.9% of
> > > the time (hence broken every so often).
> > Good point.
>
> +1
>
> > > Note that I sympathize with the amount of work involved on improving that, so
> > > better wait to hear what more knowledgeable people have to say about this :)
> > Yes, I agree.  I want to avoid coding and testing one solution only to
> > have a different reviewer NAK it.
>
> It's a pretty safe bet that everyone will prefer one code path over 2.
>
> Rob
Thanks for  the input.  Will do, and send out v3 ASAP.
Thanks, Jim

^ permalink raw reply

* Re: [PATCH v3 1/4] dt-bindings: clk: Add Baikal-T1 CCU PLLs binding
From: Rob Herring @ 2020-05-29 17:54 UTC (permalink / raw)
  To: Serge Semin
  Cc: Arnd Bergmann, linux-clk, linux-kernel, linux-mips,
	Thomas Bogendoerfer, Serge Semin, Rob Herring, Michael Turquette,
	devicetree, Alexey Malahov, Stephen Boyd
In-Reply-To: <20200526222056.18072-2-Sergey.Semin@baikalelectronics.ru>

On Wed, 27 May 2020 01:20:53 +0300, Serge Semin wrote:
> Baikal-T1 Clocks Control Unit is responsible for transformation of a
> signal coming from an external oscillator into clocks of various
> frequencies to propagate them then to the corresponding clocks
> consumers (either individual IP-blocks or clock domains). In order
> to create a set of high-frequency clocks the external signal is
> firstly handled by the embedded into CCU PLLs. So the corresponding
> dts-node is just a normal clock-provider node with standard set of
> properties. Note as being part of the Baikal-T1 System Controller its
> DT node is supposed to be a child the system controller node.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org
> 
> ---
> 
> Changelog v2:
> - Rearrange the SoBs.
> - Discard comments in the bindings file header.
> - Add dual GPL/BSD license.
> - Add spaces around the ASCII-graphics in the binding description.
> - Remove reference to Documentation/devicetree/bindings/clock/clock-bindings.txt
>   file.
> - Discard redundant object check against "/schemas/clock/clock.yaml#" schema.
> - Discard redundant descriptions of the "#clock-cells" property.
> - Remove "reg" property since from now the clock DT node is supposed to be
>   a child of the syscon-compatible system controller node.
> - Remove "clock-output-names" property support.
> - Replace "additionalProperties: false" with "unevaluatedProperties: false".
> - Lowercase the nodes name in the examples.
> - Use "clock-controller" node name suffix in the examples.
> - Remove unnecessary comments in the clocks dt-bindings header file.
> 
> Changelog v3:
> - Get the reg property back even though the driver is using the parental
>   syscon regmap.
> - The DT schema will live separately from the system controller, but the
>   corresponding sub-node of the later DT schema will $ref this one.
> ---
>  .../bindings/clock/baikal,bt1-ccu-pll.yaml    | 131 ++++++++++++++++++
>  include/dt-bindings/clock/bt1-ccu.h           |  16 +++
>  2 files changed, 147 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/baikal,bt1-ccu-pll.yaml
>  create mode 100644 include/dt-bindings/clock/bt1-ccu.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v3 2/4] dt-bindings: clk: Add Baikal-T1 CCU Dividers binding
From: Rob Herring @ 2020-05-29 17:55 UTC (permalink / raw)
  To: Serge Semin
  Cc: linux-clk, linux-kernel, devicetree, Thomas Bogendoerfer,
	Alexey Malahov, Philipp Zabel, Stephen Boyd, Arnd Bergmann,
	linux-mips, Michael Turquette, Rob Herring, Serge Semin
In-Reply-To: <20200526222056.18072-3-Sergey.Semin@baikalelectronics.ru>

On Wed, 27 May 2020 01:20:54 +0300, Serge Semin wrote:
> After being gained by the CCU PLLs the signals must be transformed to
> be suitable for the clock-consumers. This is done by a set of dividers
> embedded into the CCU. A first block of dividers is used to create
> reference clocks for AXI-bus of high-speed peripheral IP-cores of the
> chip. The second block dividers alter the PLLs output signals to be then
> consumed by SoC peripheral devices. Both block DT nodes are ordinary
> clock-providers with standard set of properties supported. But in addition
> to that each clock provider can be used to reset the corresponding clock
> domain. This makes the AXI-bus and System Devices CCU DT nodes to be also
> reset-providers.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org
> 
> ---
> 
> Changelog v2:
> - Rearrange the SoBs.
> - Combine AXI-bus and System Devices CCU bindings into a single file.
> - Discard comments in the bindings file header.
> - Add dual GPL/BSD license.
> - Add spaces around the ASCII-graphics in the binding description.
> - Remove reference to Documentation/devicetree/bindings/clock/clock-bindings.txt
>   file.
> - Discard redundant object check against "/schemas/clock/clock.yaml#" schema.
> - Discard redundant descriptions of "#clock-cells" and "#reset-cells"
>   properties.
> - Discard "reg" property since the CCU dividers DT nodes are supposed be
>   children of the syscon-compatible system controller node.
> - Remove "clock-output-names" property support.
> - Replace "additionalProperties: false" with "unevaluatedProperties: false".
> - Lowercase the nodes name in the examples.
> - Use "clock-controller" node name suffix in the examples.
> - Remove unnecessary comments in the clocks and resets dt-binding header
>   files.
> - Discard label definitions in the examples.
> 
> Changelog v3:
> - Get the reg property back even though the driver is using the parental
>   syscon regmap.
> - The DT schema will live separately from the system controller, but the
>   corresponding sub-node of the later DT schema will $ref this one.
> ---
>  .../bindings/clock/baikal,bt1-ccu-div.yaml    | 188 ++++++++++++++++++
>  include/dt-bindings/clock/bt1-ccu.h           |  32 +++
>  include/dt-bindings/reset/bt1-ccu.h           |  25 +++
>  3 files changed, 245 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/baikal,bt1-ccu-div.yaml
>  create mode 100644 include/dt-bindings/reset/bt1-ccu.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 00/14] PCI: brcmstb: enable PCIe for STB chips
From: Jim Quinlan @ 2020-05-29 17:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
	Christoph Hellwig, Nicolas Saenz Julienne,
	maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Alan Stern,
	Andy Shevchenko, Corey Minyard, Dan Williams,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE,
	Greg Kroah-Hartman, Heikki Krogerus,
	open list:DMA MAPPING HELPERS, Julien Grall,
	moderated list:ARM PORT,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:USB SUBSYSTEM, Mark Brown, Oliver Neukum,
	Rafael J. Wysocki, Robin Murphy, Saravana Kannan,
	Stefano Stabellini, Suzuki K Poulose, Ulf Hansson, Wolfram Sang
In-Reply-To: <20200529174858.GA2640397@bogus>

On Fri, May 29, 2020 at 1:49 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, May 26, 2020 at 03:12:39PM -0400, Jim Quinlan wrote:
> > v2:
> > Commit: "device core: Add ability to handle multiple dma offsets"
> >   o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
> >   o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
> >   o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
> >   o dev->dma_pfn_map => dev->dma_pfn_offset_map
> >   o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
> >   o In device.h: s/const void */const struct dma_pfn_offset_region */
> >   o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
> >     guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
> >   o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
> >     dev->dma_pfn_offset_map is copied as well.
> >   o Merged two of the DMA commits into one (Christoph).
> >
> > Commit "arm: dma-mapping: Invoke dma offset func if needed":
> >   o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
> >
> > Other commits' changes:
> >   o Removed need for carrying of_id var in priv (Nicolas)
> >   o Commit message rewordings (Bjorn)
> >   o Commit log messages filled to 75 chars (Bjorn)
> >   o devm_reset_control_get_shared())
> >     => devm_reset_control_get_optional_shared (Philipp)
> >   o Add call to reset_control_assert() in PCIe remove routines (Philipp)
> >
> > v1:
> > This patchset expands the usefulness of the Broadcom Settop Box PCIe
> > controller by building upon the PCIe driver used currently by the
> > Raspbery Pi.  Other forms of this patchset were submitted by me years
> > ago and not accepted; the major sticking point was the code required
> > for the DMA remapping needed for the PCIe driver to work [1].
> >
> > There have been many changes to the DMA and OF subsystems since that
> > time, making a cleaner and less intrusive patchset possible.  This
> > patchset implements a generalization of "dev->dma_pfn_offset", except
> > that instead of a single scalar offset it provides for multiple
> > offsets via a function which depends upon the "dma-ranges" property of
> > the PCIe host controller.  This is required for proper functionality
> > of the BrcmSTB PCIe controller and possibly some other devices.
>
> If you can enable the h/w support without the multiple offset support,
> then I'd split up this series. The latter part might take a bit more
> time.
>
> Rob
Unfortunately, the STB PCIe  controller depends on the multiple PFN
offset functionality.
Thanks,
Jim

^ permalink raw reply

* Re: [PATCH v3 2/6] hwmon: (pmbus/core) Add Block WR
From: Guenter Roeck @ 2020-05-29 17:55 UTC (permalink / raw)
  To: alexandru.tachici, linux-hwmon, linux-kernel, devicetree; +Cc: robh+dt
In-Reply-To: <20200529130506.73511-3-alexandru.tachici@analog.com>

On 5/29/20 6:05 AM, alexandru.tachici@analog.com wrote:
> From: Alexandru Tachici <alexandru.tachici@analog.com>
> 
> PmBus devices support Block Write-Block Read Process
> Call described in SMBus specification v 2.0 with the
> exception that Block writes and reads are permitted to
> have up 255 data bytes instead of max 32 bytes (SMBus).
> 
> This patch adds Block WR process call support for PMBus.
> 

I don't think I want to have this code in the PMBus core.
We can move it there if needed at some point in the future,
but for the time being I'd rather have it in the driver
needing it.

> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>
> ---
>  drivers/hwmon/pmbus/Kconfig      |  2 +-
>  drivers/hwmon/pmbus/pmbus.h      |  4 ++
>  drivers/hwmon/pmbus/pmbus_core.c | 88 ++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 6949483aa732..f11712fdcea8 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -5,7 +5,7 @@
>  
>  menuconfig PMBUS
>  	tristate "PMBus support"
> -	depends on I2C
> +	depends on I2C && CRC8

This should be select CRC8, not depends.

>  	help
>  	  Say yes here if you want to enable PMBus support.
>  
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index 18e06fc6c53f..ae4e15c5dff2 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -392,6 +392,8 @@ enum pmbus_sensor_classes {
>  #define PMBUS_PHASE_VIRTUAL	BIT(30)	/* Phases on this page are virtual */
>  #define PMBUS_PAGE_VIRTUAL	BIT(31)	/* Page is virtual */
>  
> +#define PMBUS_BLOCK_MAX		255
> +
>  enum pmbus_data_format { linear = 0, direct, vid };
>  enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv };
>  
> @@ -467,6 +469,8 @@ int pmbus_read_word_data(struct i2c_client *client, int page, int phase,
>  			 u8 reg);
>  int pmbus_write_word_data(struct i2c_client *client, int page, u8 reg,
>  			  u16 word);
> +int pmbus_block_wr(struct i2c_client *client, u8 cmd, u8 w_len, u8 *data_w,
> +		   u8 *data_r);
>  int pmbus_read_byte_data(struct i2c_client *client, int page, u8 reg);
>  int pmbus_write_byte(struct i2c_client *client, int page, u8 value);
>  int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 8d321bf7d15b..ef63468da3b5 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -6,6 +6,7 @@
>   * Copyright (c) 2012 Guenter Roeck
>   */
>  
> +#include <linux/crc8.h>
>  #include <linux/debugfs.h>
>  #include <linux/kernel.h>
>  #include <linux/math64.h>
> @@ -44,6 +45,8 @@
>  
>  #define PMBUS_NAME_SIZE		24
>  
> +DECLARE_CRC8_TABLE(pmbus_crc_table);
> +
>  struct pmbus_sensor {
>  	struct pmbus_sensor *next;
>  	char name[PMBUS_NAME_SIZE];	/* sysfs sensor name */
> @@ -184,6 +187,89 @@ int pmbus_set_page(struct i2c_client *client, int page, int phase)
>  }
>  EXPORT_SYMBOL_GPL(pmbus_set_page);
>  
> +/* Block Write/Read command.

I won't accept network-style multi-line comments.

> + * @client: Handle to slave device
> + * @cmd: Byte interpreted by slave
> + * @w_len: Size of write data block; PMBus allows at most 255 bytes
> + * @data_w: byte array which will be written.
> + * @data_r: Byte array into which data will be read; big enough to hold
> + *	the data returned by the slave. PMBus allows at most 255 bytes.
> + *
> + * Different from Block Read as it sends data and waits for the slave to
> + * return a value dependent on that data. The protocol is simply a Write Block
> + * followed by a Read Block without the Read-Block command field and the
> + * Write-Block STOP bit.
> + *
> + * Returns number of bytes read or negative errno.
> + */
> +int pmbus_block_wr(struct i2c_client *client, u8 cmd, u8 w_len,

_wr is misleading, since it suggests an abbreviated _write.
Better use something like _transfer or _xfer.

> +		   u8 *data_w, u8 *data_r)
> +{
> +	u8 write_buf[PMBUS_BLOCK_MAX + 1];
> +	struct i2c_msg msgs[2] = {
> +		{
> +			.addr = client->addr,
> +			.flags = 0,
> +			.buf = write_buf,
> +			.len = w_len + 2,
> +		},
> +		{
> +			.addr = client->addr,
> +			.flags = I2C_M_RD,
> +			.len = PMBUS_BLOCK_MAX,
> +		}
> +	};
> +	u8 addr = 0;
> +	u8 crc = 0;
> +	int ret;
> +
> +	msgs[0].buf[0] = cmd;
> +	msgs[0].buf[1] = w_len;
> +	memcpy(&msgs[0].buf[2], data_w, w_len);
> +

w_len can be up to 255, meaning up to 255 + 2 bytes can be written
into write_buf. Yet, write_buf is only 256 bytes long.

> +	msgs[0].buf = i2c_get_dma_safe_msg_buf(&msgs[0], 1);
> +	if (!msgs[0].buf)
> +		return -ENOMEM;
> +
> +	msgs[1].buf = i2c_get_dma_safe_msg_buf(&msgs[1], 1);
> +	if (!msgs[1].buf) {
> +		i2c_put_dma_safe_msg_buf(msgs[0].buf, &msgs[0], false);
> +		return -ENOMEM;
> +	}
> +
> +	ret = i2c_transfer(client->adapter, msgs, 2);
> +	if (ret != 2) {
> +		dev_err(&client->dev, "I2C transfer error.");

ret may be 1, which would suggest to the caller that one byte
of data was returned. Also, I am not in favor of logging noise.

> +		goto cleanup;
> +	}
> +
> +	if (client->flags & I2C_CLIENT_PEC) {
> +		addr = i2c_8bit_addr_from_msg(&msgs[0]);
> +		crc = crc8(pmbus_crc_table, &addr, 1, crc);
> +		crc = crc8(pmbus_crc_table, msgs[0].buf,  msgs[0].len, crc);
> +
> +		addr = i2c_8bit_addr_from_msg(&msgs[1]);
> +		crc = crc8(pmbus_crc_table, &addr, 1, crc);
> +		crc = crc8(pmbus_crc_table, msgs[1].buf,  msgs[1].buf[0] + 1,
> +			   crc);
> +
> +		if (crc != msgs[1].buf[msgs[1].buf[0] + 1]) {
> +			ret = -EBADMSG;
> +			goto cleanup;
> +		}
> +	}
> +
> +	memcpy(data_r, &msgs[1].buf[1], msgs[1].buf[0]);

The length of the read buffer is 255 bytes, yet the code suggests
that up to 256 bytes can actually be received.

> +	ret = msgs[1].buf[0];
> +
> +cleanup:
> +	i2c_put_dma_safe_msg_buf(msgs[0].buf, &msgs[0], true);
> +	i2c_put_dma_safe_msg_buf(msgs[1].buf, &msgs[1], true);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pmbus_block_wr);
> +
>  int pmbus_write_byte(struct i2c_client *client, int page, u8 value)
>  {
>  	int rv;
> @@ -2522,6 +2608,8 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
>  	if (!data->groups)
>  		return -ENOMEM;
>  
> +	crc8_populate_msb(pmbus_crc_table, 0x7);
> +
>  	i2c_set_clientdata(client, data);
>  	mutex_init(&data->update_lock);
>  	data->dev = dev;
> 


^ permalink raw reply

* Re: [PATCH V7 1/3] dt-bindings: geni-se: Convert QUP geni-se bindings to YAML
From: Rob Herring @ 2020-05-29 17:57 UTC (permalink / raw)
  To: Akash Asthana
  Cc: linux-arm-msm, devicetree, linux-kernel, mgautam, rojay, skakit,
	msavaliy
In-Reply-To: <1590560864-27037-2-git-send-email-akashast@codeaurora.org>

On Wed, May 27, 2020 at 11:57:42AM +0530, Akash Asthana wrote:
> Convert QUP geni-se bindings to DT schema format using json-schema.
> 
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> Changes in V2:
>  - As per Stephen's comment corrected defintion of interrupts for UART node.
>    Any valid UART node must contain atleast 1 interrupts.
> 
> Changes in V3:
>  - As per Rob's comment, added number of reg entries for reg property.
>  - As per Rob's comment, corrected unit address to hex.
>  - As per Rob's comment, created a pattern which matches everything common
>    to geni based I2C, SPI and UART controller and then one pattern  for each.
>  - As per Rob's comment, restored original example.
> 
> Changes in V4:
>  - Resolve below compilation error reported from bot.
> 
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/
> qcom,geni-se.yaml: properties:clocks:minItems: False schema does not allow 2
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/
> qcom,geni-se.yaml: properties:clocks:maxItems: False schema does not allow 2
> Documentation/devicetree/bindings/Makefile:12: recipe for target
> 'Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dts' failed
> make[1]: *** [Documentation/devicetree/bindings/soc/qcom/
> qcom,geni-se.example.dts] Error 1
> Makefile:1263: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
> 
> Changes in V6:
>  - Added reg entry for soc@0 example node to address below warning.
> 
> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dts:22.20-60.11
> : Warning (unit_address_vs_reg): /example-0/soc@0: node has a unit name,
> but no reg or ranges property
> 
> Changes in V7:
>  - No change.
> 
>  .../devicetree/bindings/soc/qcom/qcom,geni-se.txt  |  94 ---------
>  .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml | 210 +++++++++++++++++++++
>  2 files changed, 210 insertions(+), 94 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml

Applied, thanks.

Rob

^ permalink raw reply

* Re: [PATCH V7 2/3] dt-bindings: geni-se: Add interconnect binding for GENI QUP
From: Rob Herring @ 2020-05-29 17:57 UTC (permalink / raw)
  To: Akash Asthana
  Cc: mgautam, devicetree, linux-arm-msm, rojay, linux-kernel, skakit,
	msavaliy, robh+dt
In-Reply-To: <1590560864-27037-3-git-send-email-akashast@codeaurora.org>

On Wed, 27 May 2020 11:57:43 +0530, Akash Asthana wrote:
> Add documentation for the interconnect and interconnect-names properties
> for the GENI QUP.
> 
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> Changes in V5:
>  - Add interconnect property for QUP wrapper (parent node).
>  - Add minItems = 2 for interconnect property in child nodes
> 
> Changes in V6:
>  - As per Rob's comment added minItems = 2 for interconnect-names.
> 
> Changes in V7:
>  - No change.
> 
>  .../devicetree/bindings/soc/qcom/qcom,geni-se.yaml      | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 

Applied, thanks!

^ permalink raw reply

* Re: [PATCH V7 3/3] dt-bindings: serial: Add binding for UART pin swap
From: Rob Herring @ 2020-05-29 17:57 UTC (permalink / raw)
  To: Akash Asthana
  Cc: robh+dt, rojay, linux-kernel, msavaliy, mgautam, skakit,
	devicetree, linux-arm-msm
In-Reply-To: <1590560864-27037-4-git-send-email-akashast@codeaurora.org>

On Wed, 27 May 2020 11:57:44 +0530, Akash Asthana wrote:
> Add documentation to support RX-TX & CTS-RTS GPIO pin swap in HW.
> 
> Signed-off-by: Akash Asthana <akashast@codeaurora.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> Changes in V7:
>  - As per Rob's comment, added type: boolean to properties.
> 
>  Documentation/devicetree/bindings/serial/serial.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Applied, thanks!

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx53: ppd: alarm LEDs use kernel LED interface
From: Sebastian Reichel @ 2020-05-29 18:03 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Rob Herring, devicetree, linux-kernel, kernel,
	Ian Ray, Samu Nuutamo
In-Reply-To: <20200529160204.GA6025@duo.ucw.cz>

[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]

Hi,

On Fri, May 29, 2020 at 06:02:04PM +0200, Pavel Machek wrote:
> > ping?
> 
> Well, I thought that we maybe do not need standard LEDs on medical hardware.

The discussion died and the patch was not applied :) In general
IDK how worthwhile it is to use standard LED names for them. I
suppose the number of people planning to create something like
OpenWRT for medical devices is not so big.

> > On Fri, Apr 24, 2020 at 02:44:23PM +0200, Sebastian Reichel wrote:
> > > On Fri, Apr 24, 2020 at 11:32:26AM +0200, Pavel Machek wrote:
> > > > On Thu 2020-04-16 16:51:23, Sebastian Reichel wrote:
> > > > > From: Ian Ray <ian.ray@ge.com>
> > > > > 
> > > > > Use kernel LED interface for the alarm LEDs.
> > > > 
> > > > Could we get these changes cced to LED maintainers?
> > > 
> > > Sorry, you are not turning up via get_maintainer.pl and usually
> > > subsystem maintainers are not CC'd for every DT device instance.
> > > E.g. I do not want to be always CC'd for DT board file containing
> > > a battery/charger. I'm quite surprised you want to be CC'd for
> > > them, just looking at ARM DT files there are over 1000 instances
> > > of leds.
> 
> Well, we have mess in the naming; I'd like to clear it up.

I understand.

> > > > > +		alarm1 {
> > > > > +			label = "alarm:red";
> > > > > +			gpios = <&gpio7 3 GPIO_ACTIVE_HIGH>;
> > > > > +		};
> > > > 
> > > > So... What is function of these leds, and can we get naming more
> > > > consistent with rest of the kernel?
> > > 
> > > The device is a medical patient monitor and these are alarm LEDs
> > > informing about critical device or patient status. They are
> > > referenced by their color (those are discrete LEDs, not a
> > > multi-color one) basically everywhere. The only exception is
> > > "silenced", which means that audible alarm is surpressed. I
> > > don't think we have something comparable for any of those LEDs
> > > in the mainline tree.
> 
> Actually, we have "platform:*:mute" LEDs, that could be used for
> "silenced".

I see you point, but wonder if mute is the right choice. The LED
signals a silenced alarm, which IMHO is not the same:

* The alarm silencing is temporary and system unsilences after
  1-2 minutes.
* LED is usually blinking instead of solid like a laptop mute LED
  (so that operator is aware of silenced alarm)
* Device usually cannot be put into silenced mode before the alarm
  appears
* Some medical devices still generate perodic beeps

AFAIK this is named alarm silencing by basically everyone for
medical devices. So I think naming this platfrom:*:mute would
increase the mess.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 1/4] dt-bindings: usb: Add documentation for SG trb cache size quirk
From: Rob Herring @ 2020-05-29 18:05 UTC (permalink / raw)
  To: Tejas Joglekar; +Cc: Greg Kroah-Hartman, linux-usb, devicetree, John Youn
In-Reply-To: <d91b768b3827fce611ba052aa1bcca19ac09fd75.1590415123.git.joglekar@synopsys.com>

On Wed, May 27, 2020 at 04:10:55PM +0530, Tejas Joglekar wrote:
> This commit adds the documentation for sgl-trb-cache-size-quirk, and
> snps,sgl-trb-cache-size-quirk property. These when set enables the
> quirk for XHCI driver for consolidation of sg list into a temporary
> buffer when small buffer sizes are scattered over the sg list not
> making up to MPS or total transfer size within TRB cache size with
> Synopsys xHC.
> 
> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc3.txt     | 4 ++++
>  Documentation/devicetree/bindings/usb/usb-xhci.txt | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index d03edf9d3935..0fcbaa51f66e 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -102,6 +102,10 @@ Optional properties:
>  			this and tx-thr-num-pkt-prd to a valid, non-zero value
>  			1-16 (DWC_usb31 programming guide section 1.2.3) to
>  			enable periodic ESS TX threshold.
> + - snps,sgl-trb-cache-size-quirk: enable sg list consolidation - host mode
> +			only. Set to use SG buffers of at least MPS size
> +			by consolidating smaller SG buffers list into a
> +			single buffer.
>  
>   - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
>   - snps,incr-burst-type-adjustment: Value for INCR burst type of GSBUSCFG0
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index dc025f126d71..c53eb19ae67e 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -44,6 +44,9 @@ Optional properties:
>    - quirk-broken-port-ped: set if the controller has broken port disable mechanism
>    - imod-interval-ns: default interrupt moderation interval is 5000ns
>    - phys : see usb-hcd.yaml in the current directory
> +  - sgl-trb-cache-size-quirk: set if you need to consolidate sg list into a
> +    temporary buffer when small SG buffer sizes does not make upto MPS
> +    size or total transfer size across the TRB cache size.

Still don't understand why you have 2 properties? Is this a generic 
issue for multiple XHCI controllers? If yes, you don't need the first 
one. If no, then you don't need the second one.

Really, I'd prefer neither, and this should be implied by a specific 
compatible string. Having a separate property doesn't work if you find 
this issue later on after already adding XHCI support. IOW, don't make 
users update their DT to handle a quirk.

Rob

^ permalink raw reply

* [PATCH] dt-bindings: Merge gpio-usb-b-connector with usb-connector
From: Thierry Reding @ 2020-05-29 18:06 UTC (permalink / raw)
  To: Rob Herring
  Cc: Greg Kroah-Hartman, Prashant Malani, devicetree, linux-usb,
	linux-kernel

From: Thierry Reding <treding@nvidia.com>

The binding for usb-connector is a superset of gpio-usb-b-connector. One
major difference is that gpio-usb-b-connector requires at least one of
the vbus-gpios and id-gpios properties to be specified. Merge the two
bindings by adding the compatible string combination for the GPIO USB-B
variant and an extra conditional for the required properties list to the
usb-connector.yaml file.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 .../bindings/connector/usb-connector.yaml     | 39 +++++++++++++++++--
 .../devicetree/bindings/usb/usb-conn-gpio.txt | 30 --------------
 2 files changed, 35 insertions(+), 34 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/usb/usb-conn-gpio.txt

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 03b92b6f35fa..9bd52e63c935 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -15,10 +15,15 @@ description:
 
 properties:
   compatible:
-    enum:
-      - usb-a-connector
-      - usb-b-connector
-      - usb-c-connector
+    oneOf:
+      - enum:
+          - usb-a-connector
+          - usb-b-connector
+          - usb-c-connector
+
+      - items:
+          - const: gpio-usb-b-connector
+          - const: usb-b-connector
 
   label:
     description: Symbolic name for the connector.
@@ -140,6 +145,19 @@ properties:
 required:
   - compatible
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: gpio-usb-b-connector
+    then:
+      anyOf:
+        - required:
+            - vbus-gpios
+        - required:
+            - id-gpios
+
 examples:
   # Micro-USB connector with HS lines routed via controller (MUIC).
   - |
@@ -202,3 +220,16 @@ examples:
         op-sink-microwatt = <10000000>;
       };
     };
+
+  # USB connector with GPIO control lines
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    usb {
+      connector {
+        compatible = "gpio-usb-b-connector", "usb-b-connector";
+        type = "micro";
+        id-gpios = <&pio 12 GPIO_ACTIVE_HIGH>;
+        vbus-supply = <&usb_p0_vbus>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/usb/usb-conn-gpio.txt b/Documentation/devicetree/bindings/usb/usb-conn-gpio.txt
deleted file mode 100644
index ec80641208a5..000000000000
--- a/Documentation/devicetree/bindings/usb/usb-conn-gpio.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-USB GPIO Based Connection Detection
-
-This is typically used to switch dual role mode from the USB ID pin connected
-to an input GPIO, and also used to enable/disable device mode from the USB
-Vbus pin connected to an input GPIO.
-
-Required properties:
-- compatible : should include "gpio-usb-b-connector" and "usb-b-connector".
-- id-gpios, vbus-gpios : input gpios, either one of them must be present,
-	and both can be present as well.
-	see connector/usb-connector.yaml
-
-Optional properties:
-- vbus-supply : can be present if needed when supports dual role mode.
-	see connector/usb-connector.yaml
-
-- Sub-nodes:
-	- port : can be present.
-		see graph.txt
-
-Example:
-
-&mtu3 {
-	connector {
-		compatible = "gpio-usb-b-connector", "usb-b-connector";
-		type = "micro";
-		id-gpios = <&pio 12 GPIO_ACTIVE_HIGH>;
-		vbus-supply = <&usb_p0_vbus>;
-	};
-};
-- 
2.24.1


^ permalink raw reply related

* Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: Pavel Machek @ 2020-05-29 18:07 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: robh, kernel, shawnguo, s.hauer, kernel, festevam, linux-imx,
	mchehab, Anson.Huang, agx, angus, linux-kernel, devicetree,
	linux-arm-kernel
In-Reply-To: <20200529162850.GC3709@amd>

Hi!

Plus, do we need calibration matrix for magnetometer?

Best regards,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH V6 1/5] dt-bindings: clock: add ipq6018 a53 pll compatible
From: Rob Herring @ 2020-05-29 18:08 UTC (permalink / raw)
  To: Sivaprakash Murugesan
  Cc: agross, bjorn.andersson, mturquette, sboyd, linux-arm-msm,
	linux-clk, devicetree, linux-kernel
In-Reply-To: <1590582292-13314-2-git-send-email-sivaprak@codeaurora.org>

On Wed, May 27, 2020 at 05:54:48PM +0530, Sivaprakash Murugesan wrote:
> cpus on ipq6018 are clocked by a53 pll, add device compatible for a53
> pll found on ipq6018 devices.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> * [V6]
>     re-ordered compatible string, dropped Rob's review tag for this change.

Not really significant enough to drop it, but if you really want me to 
stare at this again...

>  .../devicetree/bindings/clock/qcom,a53pll.yaml         | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> index 20d2638..a4f2d01 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,a53pll.yaml
> @@ -15,6 +15,7 @@ description:
>  
>  properties:
>    compatible:
> +    const: qcom,ipq6018-a53pll
>      const: qcom,msm8916-a53pll
>  
>    reg:
> @@ -23,6 +24,14 @@ properties:
>    '#clock-cells':
>      const: 0
>  
> +  clocks:
> +    items:
> +      - description: board XO clock
> +
> +  clock-names:
> +    items:
> +      - const: xo
> +
>  required:
>    - compatible
>    - reg
> @@ -38,3 +47,12 @@ examples:
>          reg = <0xb016000 0x40>;
>          #clock-cells = <0>;
>      };
> +  #Example 2 - A53 PLL found on IPQ6018 devices
> +  - |
> +    a53pll_ipq: clock@b116000 {

clock-controller@...

> +        compatible = "qcom,ipq6018-a53pll";
> +        reg = <0x0b116000 0x40>;
> +        #clock-cells = <0>;
> +        clocks = <&xo>;
> +        clock-names = "xo";
> +    };
> -- 
> 2.7.4
> 

^ permalink raw reply

* Re: [PATCH V6 3/5] clk: qcom: Add DT bindings for ipq6018 apss clock controller
From: Rob Herring @ 2020-05-29 18:08 UTC (permalink / raw)
  To: Sivaprakash Murugesan
  Cc: sboyd, robh+dt, agross, mturquette, linux-clk, linux-kernel,
	linux-arm-msm, devicetree, bjorn.andersson
In-Reply-To: <1590582292-13314-4-git-send-email-sivaprak@codeaurora.org>

On Wed, 27 May 2020 17:54:50 +0530, Sivaprakash Murugesan wrote:
> Add dt-binding for ipq6018 apss clock controller
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> [V6]
>  * Addressed review comment from Stephen
>  include/dt-bindings/clock/qcom,apss-ipq.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  create mode 100644 include/dt-bindings/clock/qcom,apss-ipq.h
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v5 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Rob Herring @ 2020-05-29 18:13 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Alexey Malahov,
	Thomas Bogendoerfer, Andy Shevchenko, Mika Westerberg, linux-mips,
	linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200527153351.rmzguymz7lm6gvsx@mobilestation>

On Wed, May 27, 2020 at 06:33:51PM +0300, Serge Semin wrote:
> Rob,
> Could you pay attention to this patch? The patchset review procedure is
> nearly over, while the DT part is only partly reviewed by you.

Pretty sure I commented on this. Not sure what version, you're sending 
new versions too fast. Give people time to review.

> 
> Thanks
> -Sergey
> 
> On Wed, May 27, 2020 at 06:30:37PM +0300, Serge Semin wrote:
> > dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
> > i2c "reg" property. If it is the compiler will print a warning:
> > 
> > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
> > 
> > In order to silence dtc up let's discard the flag from the DW I2C DT
> > binding example for now. Just revert this commit when dtc is fixed.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: linux-mips@vger.kernel.org
> > 
> > ---
> > 
> > Changelog v3:
> > - This is a new patch created as a result of the Rob request to remove
> >   the EEPROM-slave bit setting in the DT binndings example until the dtc
> >   is fixed.
> > ---
> >  Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > index 4bd430b2b41d..101d78e8f19d 100644
> > --- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > @@ -137,7 +137,7 @@ examples:
> >  
> >        eeprom@64 {
> >          compatible = "linux,slave-24c02";
> > -        reg = <0x40000064>;
> > +        reg = <0x64>;

This is wrong though because "linux,slave-24c02" should have bit 30 set. 
(And either the unit-address was wrong or we can define the unit-address 
does not include the high bits.)

Rob

^ permalink raw reply


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