* Re: [PATCH v2 02/19] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
From: Andy Shevchenko @ 2020-05-15 12:01 UTC (permalink / raw)
To: Serge Semin
Cc: Mark Brown, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Allison Randal, Gareth Williams, Rob Herring,
linux-mips, devicetree, Jarkko Nikula, Wan Ahmad Zainie,
Thomas Gleixner, wuxu.wu, Clement Leger, linux-spi, linux-kernel
In-Reply-To: <20200515104758.6934-3-Sergey.Semin@baikalelectronics.ru>
On Fri, May 15, 2020 at 01:47:41PM +0300, Serge Semin wrote:
> Since DMA transfers are performed asynchronously with actual SPI
> transaction, then even if DMA transfers are finished it doesn't mean
> all data is actually pushed to the SPI bus. Some data might still be
> in the controller FIFO. This is specifically true for Tx-only
> transfers. In this case if the next SPI transfer is recharged while
> a tail of the previous one is still in FIFO, we'll loose that tail
> data. In order to fix this lets add the wait procedure of the Tx/Rx
> SPI transfers completion after the corresponding DMA transactions
> are finished.
General question, doesn't spi core provides us some helpers like
spi_delay_exec()?
> Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Allison Randal <allison@lohutok.net>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Gareth Williams <gareth.williams.jx@renesas.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org
>
> ---
>
> Changelog v2:
> - Use conditional statement instead of the ternary operator in the ref
> clock getter.
> - Move the patch to the head of the series so one could be picked up to
> the stable kernels as a fix.
You forgot a Fixes tag.
> ---
> drivers/spi/spi-dw-mid.c | 50 ++++++++++++++++++++++++++++++++++++++++
> drivers/spi/spi-dw.h | 10 ++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
> index 177e1f5ec62b..7a5ae1506365 100644
> --- a/drivers/spi/spi-dw-mid.c
> +++ b/drivers/spi/spi-dw-mid.c
> @@ -16,7 +16,9 @@
> #include <linux/irqreturn.h>
> #include <linux/pci.h>
> #include <linux/platform_data/dma-dw.h>
> +#include <linux/delay.h>
Keep it in order.
>
> +#define WAIT_RETRIES 5
> #define RX_BUSY 0
> #define TX_BUSY 1
>
> @@ -141,6 +143,28 @@ static enum dma_slave_buswidth convert_dma_width(u32 dma_width) {
> return DMA_SLAVE_BUSWIDTH_UNDEFINED;
> }
>
> +static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws)
> +{
> + return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT);
> +}
> +
> +static void dw_spi_dma_wait_tx_done(struct dw_spi *dws)
> +{
> + int retry = WAIT_RETRIES;
> + unsigned long ns;
> +
> + ns = (NSEC_PER_SEC / spi_get_clk(dws)) * dws->n_bytes * BITS_PER_BYTE;
> + ns *= dw_readl(dws, DW_SPI_TXFLR);
> +
> + while (dw_spi_dma_tx_busy(dws) && retry--)
> + ndelay(ns);
This misses power management for CPU and do you really need this to be atomic?
At the end why not to use readx_poll_timeout() ?
> + if (retry < 0) {
Usually we do
unsigned int retries = NNNN;
do {
...
} while (--retries);
if (!retries)
...
But in any case, see above.
> + dev_err(&dws->master->dev, "Tx hanged up\n");
> + dws->master->cur_msg->status = -EIO;
> + }
> +}
Same comments to Rx part.
> +
> /*
> * dws->dma_chan_busy is set before the dma transfer starts, callback for tx
> * channel will clear a corresponding bit.
> @@ -149,6 +173,8 @@ static void dw_spi_dma_tx_done(void *arg)
> {
> struct dw_spi *dws = arg;
>
> + dw_spi_dma_wait_tx_done(dws);
> +
> clear_bit(TX_BUSY, &dws->dma_chan_busy);
> if (test_bit(RX_BUSY, &dws->dma_chan_busy))
> return;
> @@ -188,6 +214,28 @@ static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,
> return txdesc;
> }
>
> +static inline bool dw_spi_dma_rx_busy(struct dw_spi *dws)
> +{
> + return !!(dw_readl(dws, DW_SPI_SR) & SR_RF_NOT_EMPT);
> +}
> +
> +static void dw_spi_dma_wait_rx_done(struct dw_spi *dws)
> +{
> + int retry = WAIT_RETRIES;
> + unsigned long ns;
> +
> + ns = (NSEC_PER_SEC / spi_get_clk(dws)) * dws->n_bytes * BITS_PER_BYTE;
> + ns *= dw_readl(dws, DW_SPI_RXFLR);
> +
> + while (dw_spi_dma_rx_busy(dws) && retry--)
> + ndelay(ns);
> +
> + if (retry < 0) {
> + dev_err(&dws->master->dev, "Rx hanged up\n");
> + dws->master->cur_msg->status = -EIO;
> + }
> +}
> +
> /*
> * dws->dma_chan_busy is set before the dma transfer starts, callback for rx
> * channel will clear a corresponding bit.
> @@ -196,6 +244,8 @@ static void dw_spi_dma_rx_done(void *arg)
> {
> struct dw_spi *dws = arg;
>
> + dw_spi_dma_wait_rx_done(dws);
> +
> clear_bit(RX_BUSY, &dws->dma_chan_busy);
> if (test_bit(TX_BUSY, &dws->dma_chan_busy))
> return;
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index e92d43b9a9e6..81364f501b7e 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -210,6 +210,16 @@ static inline void spi_set_clk(struct dw_spi *dws, u16 div)
> dw_writel(dws, DW_SPI_BAUDR, div);
> }
>
> +static inline u32 spi_get_clk(struct dw_spi *dws)
> +{
> + u32 div = dw_readl(dws, DW_SPI_BAUDR);
> +
> + if (!div)
> + return 0;
> +
> + return dws->max_freq / div;
> +}
> +
> /* Disable IRQ bits */
> static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
> {
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 01/19] dt-bindings: spi: dw: Add Tx/Rx DMA properties
From: Andy Shevchenko @ 2020-05-15 11:51 UTC (permalink / raw)
To: Serge Semin
Cc: Mark Brown, Rob Herring, Serge Semin, Georgy Vlasov,
Ramil Zaripov, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
Ralf Baechle, Arnd Bergmann, Allison Randal, Gareth Williams,
linux-mips, Wan Ahmad Zainie, linux-spi, devicetree, linux-kernel
In-Reply-To: <20200515104758.6934-2-Sergey.Semin@baikalelectronics.ru>
On Fri, May 15, 2020 at 01:47:40PM +0300, Serge Semin wrote:
> Since commit 22d48ad7bfac ("spi: dw: Add Elkhart Lake PSE DMA support")
> the spi-dw-mid.c module supports a platform DMA engine handling the DW APB
> SSI controller requests. Lets alter the DW SPI bindings file to accept the
> Rx and Tx DMA line specifiers.
I'm wondering if these properties are implied by the SPI generic one?
(forgive me if I'm not understanding all DT schema relations)
Per se looks good.
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Allison Randal <allison@lohutok.net>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Gareth Williams <gareth.williams.jx@renesas.com>
> Cc: linux-mips@vger.kernel.org
>
> ---
>
> Changelog v2:
> - Revert the order of the DT changes: first add the DMA channels support,
> then perform the binding file conversion.
> ---
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index 7a4702edf896..020e3168ee41 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -23,6 +23,8 @@ Optional properties:
> - num-cs : The number of chipselects. If omitted, this will default to 4.
> - reg-io-width : The I/O register width (in bytes) implemented by this
> device. Supported values are 2 or 4 (the default).
> +- dmas : Phandle + identifiers of Tx and Rx DMA channels.
> +- dma-names : Contains the names of the DMA channels. Must be "tx" and "rx".
>
> Child nodes as per the generic SPI binding.
>
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 00/19] spi: dw: Add generic DW DMA controller support
From: Andy Shevchenko @ 2020-05-15 11:49 UTC (permalink / raw)
To: Serge Semin
Cc: Mark Brown, Serge Semin, Georgy Vlasov, Ramil Zaripov,
Alexey Malahov, Maxim Kaurkin, Pavel Parkhomenko,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
Thomas Bogendoerfer, Paul Burton, Ralf Baechle, Arnd Bergmann,
Allison Randal, Gareth Williams, Rob Herring, linux-mips,
linux-spi, devicetree, linux-kernel
In-Reply-To: <20200515104758.6934-1-Sergey.Semin@baikalelectronics.ru>
On Fri, May 15, 2020 at 01:47:39PM +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.
>
> First of all we need to add the Tx and Rx DMA channels support into the DW
> APB SSI binding. Then there are several fixes and cleanups provided as a
> initial preparation for the Generic DMA support integration: add Tx/Rx
> finish wait methods, clear DMAC register when done or stopped, Fix native
> CS being unset, enable interrupts in accordance with DMA xfer mode,
> discard static DW DMA slave structures, discard unused void priv pointer
> and dma_width member of the dw_spi structure, provide the DMA Tx/Rx burst
> length parametrisation and make sure it's optionally set in accordance
> with the DMA max-burst capability.
>
> In order to have the DW APB SSI MMIO driver working with DMA we need to
> initialize the paddr field with the physical base address of the DW APB SSI
> registers space. Then we unpin the Intel MID specific code from the
> generic DMA one and placed it into the spi-dw-pci.c driver, which is a
> better place for it anyway. After that the naming cleanups are performed
> since the code is going to be used for a generic DMAC device. Finally the
> Generic DMA initialization can be added to the generic version of the
> DW APB SSI IP.
>
> Last but not least we traditionally convert the legacy plain text-based
> dt-binding file with yaml-based one and as a cherry on a cake replace
> the manually written DebugFS registers read method with a ready-to-use
> for the same purpose regset32 DebugFS interface usage.
>
> This patchset is rebased and tested on the spi/for-next (5.7-rc5):
> base-commit: fe9fce6b2cf3 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")
Thanks! I'm going to review it soon.
Hint for the next time, please start always a new thread with new version.
> Co-developed-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Signed-off-by: Georgy Vlasov <Georgy.Vlasov@baikalelectronics.ru>
> Co-developed-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
> Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
> Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
> Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Allison Randal <allison@lohutok.net>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Gareth Williams <gareth.williams.jx@renesas.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: linux-spi@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> ---
>
> Changelog v2:
> - Rebase on top of the spi repository for-next branch.
> - Move bindings conversion patch to the tail of the series.
> - Move fixes to the head of the series.
> - Apply as many changes as possible to be applied the Generic DMA
> functionality support is added and the spi-dw-mid is moved to the
> spi-dw-dma driver.
> - Discard patch "spi: dw: Fix dma_slave_config used partly uninitialized"
> since the problem has already been fixed.
> - Add new patch "spi: dw: Discard unused void priv pointer".
> - Add new patch "spi: dw: Discard dma_width member of the dw_spi structure".
> n_bytes member of the DW SPI data can be used instead.
> - Build the DMA functionality into the DW APB SSI core if required instead
> of creating a separate kernel module.
> - Use conditional statement instead of the ternary operator in the ref
> clock getter.
>
> Serge Semin (19):
> dt-bindings: spi: dw: Add Tx/Rx DMA properties
> spi: dw: Add Tx/Rx finish wait methods to the MID DMA
> spi: dw: Clear DMAC register when done or stopped
> spi: dw: Fix native CS being unset
> spi: dw: Enable interrupts in accordance with DMA xfer mode
> spi: dw: Discard static DW DMA slave structures
> spi: dw: Discard unused void priv pointer
> spi: dw: Discard dma_width member of the dw_spi structure
> spi: dw: Parameterize the DMA Rx/Tx burst length
> spi: dw: Use DMA max burst to set the request thresholds
> spi: dw: Initialize paddr in DW SPI MMIO private data
> spi: dw: Fix Rx-only DMA transfers
> spi: dw: Move Non-DMA code to the DW PCIe-SPI driver
> spi: dw: Remove DW DMA code dependency from DW_DMAC_PCI
> spi: dw: Add DW SPI DMA/PCI/MMIO dependency on the DW SPI core
> spi: dw: Cleanup generic DW DMA code namings
> spi: dw: Add DMA support to the DW SPI MMIO driver
> spi: dw: Use regset32 DebugFS method to create regdump file
> dt-bindings: spi: Convert DW SPI binding to DT schema
>
> .../bindings/spi/snps,dw-apb-ssi.txt | 42 ---
> .../bindings/spi/snps,dw-apb-ssi.yaml | 127 +++++++++
> .../devicetree/bindings/spi/spi-dw.txt | 24 --
> drivers/spi/Kconfig | 15 +-
> drivers/spi/Makefile | 7 +-
> drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} | 257 ++++++++++--------
> drivers/spi/spi-dw-mmio.c | 9 +-
> drivers/spi/spi-dw-pci.c | 50 +++-
> drivers/spi/spi-dw.c | 98 +++----
> drivers/spi/spi-dw.h | 33 ++-
> 10 files changed, 405 insertions(+), 257 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> create mode 100644 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> delete mode 100644 Documentation/devicetree/bindings/spi/spi-dw.txt
> rename drivers/spi/{spi-dw-mid.c => spi-dw-dma.c} (53%)
>
> --
> 2.25.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 7/7] iio: accel: Add bma150/smb380 support to bma180
From: Linus Walleij @ 2020-05-15 11:49 UTC (permalink / raw)
To: Jonathan Bakker
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Rob Herring, Kate Stewart, Thomas Gleixner,
linux-iio,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdbucZ68KODd3shecm0uEkFgTzh4XiN08JdCM_chYApoXA@mail.gmail.com>
On Fri, May 15, 2020 at 1:48 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, May 14, 2020 at 10:49 PM Jonathan Bakker <xc-racer2@live.ca> wrote:
>
> > The bma150/smb380 are very similar to the bma023 but have a temperature
> > channel as well.
> >
> > Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
>
> The temperature channel can be added later, let's go ahead with this!
Oh I see you added it, missing things ... thanks Jonathan.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 7/7] iio: accel: Add bma150/smb380 support to bma180
From: Linus Walleij @ 2020-05-15 11:48 UTC (permalink / raw)
To: Jonathan Bakker
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Rob Herring, Kate Stewart, Thomas Gleixner,
linux-iio,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <BN6PR04MB0660C7BA4CE07978AEC884E1A3BC0@BN6PR04MB0660.namprd04.prod.outlook.com>
On Thu, May 14, 2020 at 10:49 PM Jonathan Bakker <xc-racer2@live.ca> wrote:
> The bma150/smb380 are very similar to the bma023 but have a temperature
> channel as well.
>
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
The temperature channel can be added later, let's go ahead with this!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
From: Andy Shevchenko @ 2020-05-15 11:48 UTC (permalink / raw)
To: Ramuthevar,Vadivel MuruganX
Cc: linux-kernel, linux-mtd, devicetree, miquel.raynal, richard,
vigneshr, arnd, brendanhiggins, tglx, boris.brezillon,
anders.roxell, masonccyang, robh+dt, linux-mips, hauke.mehrtens,
qi-ming.wu, cheol.yong.kim
In-Reply-To: <20200515105537.4876-3-vadivel.muruganx.ramuthevar@linux.intel.com>
On Fri, May 15, 2020 at 06:55:37PM +0800, Ramuthevar,Vadivel MuruganX wrote:
> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>
> This patch adds the new IP of Nand Flash Controller(NFC) support
> on Intel's Lightning Mountain(LGM) SoC.
>
> DMA is used for burst data transfer operation, also DMA HW supports
> aligned 32bit memory address and aligned data access by default.
> DMA burst of 8 supported. Data register used to support the read/write
> operation from/to device.
>
> NAND controller driver implements ->exec_op() to replace legacy hooks,
> these specific call-back method to execute NAND operations.
...
> + ebu_host->dma_tx = dma_request_chan(dev, "tx");
> + if (IS_ERR(ebu_host->dma_tx)) {
> + ret = PTR_ERR(ebu_host->dma_tx);
> + dev_err(dev, "DMA tx channel request fail!.\n");
> + goto err_cleanup_dma;
> + }
> +
> + ebu_host->dma_rx = dma_request_chan(dev, "rx");
> + if (IS_ERR(ebu_host->dma_rx)) {
> + ret = PTR_ERR(ebu_host->dma_rx);
> + dev_err(dev, "DMA tx channel request fail!.\n");
rx ?
> + goto err_cleanup_dma;
> + }
...
> +static int ebu_nand_remove(struct platform_device *pdev)
> +{
> + struct ebu_nand_controller *ebu_host = platform_get_drvdata(pdev);
> +
> + if (ebu_host) {
I dunno why you need this check? Maybe I forgot your answer to my comment?
> + mtd_device_unregister(nand_to_mtd(&ebu_host->chip));
> + nand_cleanup(&ebu_host->chip);
> + ebu_nand_disable(&ebu_host->chip);
> + ebu_dma_cleanup(ebu_host);
> + clk_disable_unprepare(ebu_host->clk);
> + }
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 6/7] iio: accel: bma180: Rename center_temp to temp_offset
From: Linus Walleij @ 2020-05-15 11:47 UTC (permalink / raw)
To: Jonathan Bakker
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Rob Herring, Kate Stewart, Thomas Gleixner,
linux-iio,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <BN6PR04MB06603F08B010828F7CBFB610A3BC0@BN6PR04MB0660.namprd04.prod.outlook.com>
On Thu, May 14, 2020 at 10:49 PM Jonathan Bakker <xc-racer2@live.ca> wrote:
> The bma180 driver is being extended to support the bma150.
> Its temperature channel is unsigned so the center_temp naming
> no longer makes.
>
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 2/7] iio: accel: Make bma180 conflict with input's bma150
From: Linus Walleij @ 2020-05-15 11:46 UTC (permalink / raw)
To: Jonathan Bakker
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Rob Herring, Kate Stewart, Thomas Gleixner,
linux-iio,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org
In-Reply-To: <BN6PR04MB0660FE5CCD136539A328B29EA3BC0@BN6PR04MB0660.namprd04.prod.outlook.com>
On Thu, May 14, 2020 at 10:49 PM Jonathan Bakker <xc-racer2@live.ca> wrote:
> The bma180 IIO driver is being extended for support for the chips
> support by input's bma150 driver (bma023, bma150, smb380). Don't
> allow both drivers to be enabled simultaneously as they're for the
> same hardware.
>
> Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH V1 3/3] mmc: sdhci: Allow platform controlled voltage switching
From: Veerabhadrarao Badiganti @ 2020-05-15 11:18 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Vijay Viswanath, Veerabhadrarao Badiganti
In-Reply-To: <1589541535-8523-1-git-send-email-vbadigan@codeaurora.org>
From: Vijay Viswanath <vviswana@codeaurora.org>
If vendor platform drivers are controlling whole logic of voltage
switching, then sdhci driver no need control vqmmc regulator.
So skip enabling/disable vqmmc from SDHC driver.
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
drivers/mmc/host/sdhci.c | 32 +++++++++++++++++++-------------
drivers/mmc/host/sdhci.h | 1 +
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1bb6b67..c010823 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -4098,6 +4098,7 @@ int sdhci_setup_host(struct sdhci_host *host)
unsigned int override_timeout_clk;
u32 max_clk;
int ret;
+ bool enable_vqmmc = false;
WARN_ON(host == NULL);
if (host == NULL)
@@ -4111,9 +4112,12 @@ int sdhci_setup_host(struct sdhci_host *host)
* the host can take the appropriate action if regulators are not
* available.
*/
- ret = mmc_regulator_get_supply(mmc);
- if (ret)
- return ret;
+ if (!mmc->supply.vqmmc) {
+ ret = mmc_regulator_get_supply(mmc);
+ if (ret)
+ return ret;
+ enable_vqmmc = true;
+ }
DBG("Version: 0x%08x | Present: 0x%08x\n",
sdhci_readw(host, SDHCI_HOST_VERSION),
@@ -4373,7 +4377,15 @@ int sdhci_setup_host(struct sdhci_host *host)
mmc->caps |= MMC_CAP_NEEDS_POLL;
if (!IS_ERR(mmc->supply.vqmmc)) {
- ret = regulator_enable(mmc->supply.vqmmc);
+ if (enable_vqmmc) {
+ ret = regulator_enable(mmc->supply.vqmmc);
+ if (ret) {
+ pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
+ mmc_hostname(mmc), ret);
+ mmc->supply.vqmmc = ERR_PTR(-EINVAL);
+ }
+ host->vqmmc_enabled = !ret;
+ }
/* If vqmmc provides no 1.8V signalling, then there's no UHS */
if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
@@ -4386,12 +4398,6 @@ int sdhci_setup_host(struct sdhci_host *host)
if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 2700000,
3600000))
host->flags &= ~SDHCI_SIGNALING_330;
-
- if (ret) {
- pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
- mmc_hostname(mmc), ret);
- mmc->supply.vqmmc = ERR_PTR(-EINVAL);
- }
}
if (host->quirks2 & SDHCI_QUIRK2_NO_1_8_V) {
@@ -4625,7 +4631,7 @@ int sdhci_setup_host(struct sdhci_host *host)
return 0;
unreg:
- if (!IS_ERR(mmc->supply.vqmmc))
+ if (host->vqmmc_enabled)
regulator_disable(mmc->supply.vqmmc);
undma:
if (host->align_buffer)
@@ -4643,7 +4649,7 @@ void sdhci_cleanup_host(struct sdhci_host *host)
{
struct mmc_host *mmc = host->mmc;
- if (!IS_ERR(mmc->supply.vqmmc))
+ if (host->vqmmc_enabled)
regulator_disable(mmc->supply.vqmmc);
if (host->align_buffer)
@@ -4780,7 +4786,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
destroy_workqueue(host->complete_wq);
- if (!IS_ERR(mmc->supply.vqmmc))
+ if (host->vqmmc_enabled)
regulator_disable(mmc->supply.vqmmc);
if (host->align_buffer)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 8d2a096..24d27e1 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -570,6 +570,7 @@ struct sdhci_host {
u32 caps1; /* CAPABILITY_1 */
bool read_caps; /* Capability flags have been read */
+ bool vqmmc_enabled; /* Vqmmc is enabled */
unsigned int ocr_avail_sdio; /* OCR bit masks */
unsigned int ocr_avail_sd;
unsigned int ocr_avail_mmc;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V1 2/3] mmc: sdhci-msm: Use internal voltage control
From: Veerabhadrarao Badiganti @ 2020-05-15 11:18 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Vijay Viswanath, Asutosh Das, Veerabhadrarao Badiganti,
Andy Gross, Bjorn Andersson
In-Reply-To: <1589541535-8523-1-git-send-email-vbadigan@codeaurora.org>
From: Vijay Viswanath <vviswana@codeaurora.org>
On qcom SD host controllers voltage switching be done after the HW
is ready for it. The HW informs its readiness through power irq.
The voltage switching should happen only then.
Use the internal voltage switching and then control the voltage
switching using power irq.
Set the regulator load as well so that regulator can be configured
in LPM mode when in is not being used.
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
drivers/mmc/host/sdhci-msm.c | 215 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 207 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 97758fa..a10e955 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -36,7 +36,9 @@
#define CORE_PWRCTL_IO_LOW BIT(2)
#define CORE_PWRCTL_IO_HIGH BIT(3)
#define CORE_PWRCTL_BUS_SUCCESS BIT(0)
+#define CORE_PWRCTL_BUS_FAIL BIT(1)
#define CORE_PWRCTL_IO_SUCCESS BIT(2)
+#define CORE_PWRCTL_IO_FAIL BIT(3)
#define REQ_BUS_OFF BIT(0)
#define REQ_BUS_ON BIT(1)
#define REQ_IO_LOW BIT(2)
@@ -263,6 +265,9 @@ struct sdhci_msm_host {
bool use_cdr;
u32 transfer_mode;
bool updated_ddr_cfg;
+ u32 vmmc_load;
+ u32 vqmmc_load;
+ bool vqmmc_enabled;
};
static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -1298,6 +1303,78 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
sdhci_msm_hs400(host, &mmc->ios);
}
+static int sdhci_msm_set_vmmc(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, int level)
+{
+ int load, ret;
+
+ if (IS_ERR(mmc->supply.vmmc))
+ return 0;
+
+ if (msm_host->vmmc_load) {
+ load = level ? msm_host->vmmc_load : 0;
+ ret = regulator_set_load(mmc->supply.vmmc, load);
+ if (ret)
+ goto out;
+ }
+
+ ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
+out:
+ if (ret)
+ pr_err("%s: vmmc set load/ocr failed: %d\n",
+ mmc_hostname(mmc), ret);
+
+ return ret;
+}
+
+static int sdhci_msm_set_vqmmc(struct sdhci_msm_host *msm_host,
+ struct mmc_host *mmc, int level)
+{
+ int load, ret;
+ struct mmc_ios ios;
+
+ if (IS_ERR(mmc->supply.vqmmc) ||
+ (mmc->ios.power_mode == MMC_POWER_UNDEFINED) ||
+ (msm_host->vqmmc_enabled == level))
+ return 0;
+
+ if (msm_host->vqmmc_load) {
+ load = level ? msm_host->vqmmc_load : 0;
+ ret = regulator_set_load(mmc->supply.vqmmc, load);
+ if (ret)
+ goto out;
+ }
+
+ /*
+ * The IO voltage regulator may not always support a voltage close to
+ * vdd. Set IO voltage based on capability of the regulator.
+ */
+ if (level) {
+ if (msm_host->caps_0 & CORE_3_0V_SUPPORT)
+ ios.signal_voltage = MMC_SIGNAL_VOLTAGE_330;
+ else if (msm_host->caps_0 & CORE_1_8V_SUPPORT)
+ ios.signal_voltage = MMC_SIGNAL_VOLTAGE_180;
+ if (msm_host->caps_0 & CORE_VOLT_SUPPORT) {
+ pr_debug("%s: %s: setting signal voltage: %d\n",
+ mmc_hostname(mmc), __func__,
+ ios.signal_voltage);
+ ret = mmc_regulator_set_vqmmc(mmc, &ios);
+ if (ret < 0)
+ goto out;
+ }
+ ret = regulator_enable(mmc->supply.vqmmc);
+ } else {
+ ret = regulator_disable(mmc->supply.vqmmc);
+ }
+out:
+ if (ret)
+ pr_err("%s: vqmmc failed: %d\n", mmc_hostname(mmc), ret);
+ else
+ msm_host->vqmmc_enabled = level;
+
+ return ret;
+}
+
static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
{
init_waitqueue_head(&msm_host->pwr_irq_wait);
@@ -1401,8 +1478,9 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+ struct mmc_host *mmc = host->mmc;
u32 irq_status, irq_ack = 0;
- int retry = 10;
+ int retry = 10, ret = 0;
u32 pwr_state = 0, io_level = 0;
u32 config;
const struct sdhci_msm_offset *msm_offset = msm_host->offset;
@@ -1438,14 +1516,35 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
/* Handle BUS ON/OFF*/
if (irq_status & CORE_PWRCTL_BUS_ON) {
- pwr_state = REQ_BUS_ON;
- io_level = REQ_IO_HIGH;
- irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+ ret = sdhci_msm_set_vmmc(msm_host, mmc, 1);
+ if (!ret)
+ ret = sdhci_msm_set_vqmmc(msm_host, mmc, 1);
+
+ if (!ret) {
+ pwr_state = REQ_BUS_ON;
+ io_level = REQ_IO_HIGH;
+ irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+ } else {
+ pr_err("%s: BUS_ON req failed(%d). irq_status: 0x%08x\n",
+ mmc_hostname(mmc), ret, irq_status);
+ irq_ack |= CORE_PWRCTL_BUS_FAIL;
+ sdhci_msm_set_vmmc(msm_host, mmc, 0);
+ }
}
if (irq_status & CORE_PWRCTL_BUS_OFF) {
- pwr_state = REQ_BUS_OFF;
- io_level = REQ_IO_LOW;
- irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+ ret = sdhci_msm_set_vmmc(msm_host, mmc, 0);
+ if (!ret)
+ ret = sdhci_msm_set_vqmmc(msm_host, mmc, 0);
+
+ if (!ret) {
+ pwr_state = REQ_BUS_OFF;
+ io_level = REQ_IO_LOW;
+ irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
+ } else {
+ pr_err("%s: BUS_ON req failed(%d). irq_status: 0x%08x\n",
+ mmc_hostname(mmc), ret, irq_status);
+ irq_ack |= CORE_PWRCTL_BUS_FAIL;
+ }
}
/* Handle IO LOW/HIGH */
if (irq_status & CORE_PWRCTL_IO_LOW) {
@@ -1457,6 +1556,15 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
irq_ack |= CORE_PWRCTL_IO_SUCCESS;
}
+ if (io_level && !IS_ERR(mmc->supply.vqmmc) && !pwr_state) {
+ ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
+ if (ret < 0)
+ pr_err("%s: IO_level setting failed(%d). signal_voltage: %d, vdd: %d irq_status: 0x%08x\n",
+ mmc_hostname(mmc), ret,
+ mmc->ios.signal_voltage, mmc->ios.vdd,
+ irq_status);
+ }
+
/*
* The driver has to acknowledge the interrupt, switch voltages and
* report back if it succeded or not to this register. The voltage
@@ -1833,6 +1941,91 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
sdhci_reset(host, mask);
}
+static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
+{
+ int ret = 0;
+ struct mmc_host *mmc = msm_host->mmc;
+
+ ret = mmc_regulator_get_supply(msm_host->mmc);
+ if (ret)
+ return ret;
+ device_property_read_u32(&msm_host->pdev->dev,
+ "vmmc-max-load-microamp",
+ &msm_host->vmmc_load);
+ device_property_read_u32(&msm_host->pdev->dev,
+ "vqmmc-max-load-microamp",
+ &msm_host->vqmmc_load);
+
+ sdhci_msm_set_regulator_caps(msm_host);
+ mmc->ios.power_mode = MMC_POWER_UNDEFINED;
+
+ return 0;
+
+}
+
+static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
+ struct mmc_ios *ios)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ u16 ctrl;
+
+ /*
+ * Signal Voltage Switching is only applicable for Host Controllers
+ * v3.00 and above.
+ */
+ if (host->version < SDHCI_SPEC_300)
+ return 0;
+
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+
+ switch (ios->signal_voltage) {
+ case MMC_SIGNAL_VOLTAGE_330:
+ if (!(host->flags & SDHCI_SIGNALING_330))
+ return -EINVAL;
+ /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
+ ctrl &= ~SDHCI_CTRL_VDD_180;
+ sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+ /* 3.3V regulator output should be stable within 5 ms */
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ if (!(ctrl & SDHCI_CTRL_VDD_180))
+ return 0;
+
+ pr_warn("%s: 3.3V regulator output did not became stable\n",
+ mmc_hostname(mmc));
+
+ return -EAGAIN;
+ case MMC_SIGNAL_VOLTAGE_180:
+ if (!(host->flags & SDHCI_SIGNALING_180))
+ return -EINVAL;
+
+ /*
+ * Enable 1.8V Signal Enable in the Host Control2
+ * register
+ */
+ ctrl |= SDHCI_CTRL_VDD_180;
+ sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+ /* 1.8V regulator output should be stable within 5 ms */
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ if (ctrl & SDHCI_CTRL_VDD_180)
+ return 0;
+
+ pr_warn("%s: 1.8V regulator output did not became stable\n",
+ mmc_hostname(mmc));
+
+ return -EAGAIN;
+ case MMC_SIGNAL_VOLTAGE_120:
+ if (!(host->flags & SDHCI_SIGNALING_120))
+ return -EINVAL;
+ return 0;
+ default:
+ /* No signal voltage switch required */
+ return 0;
+ }
+
+}
+
static const struct sdhci_msm_variant_ops mci_var_ops = {
.msm_readl_relaxed = sdhci_msm_mci_variant_readl_relaxed,
.msm_writel_relaxed = sdhci_msm_mci_variant_writel_relaxed,
@@ -1880,6 +2073,7 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
.write_w = sdhci_msm_writew,
.write_b = sdhci_msm_writeb,
.irq = sdhci_msm_cqe_irq,
+ .set_power = sdhci_set_power_noreg,
};
static const struct sdhci_pltfm_data sdhci_msm_pdata = {
@@ -2072,6 +2266,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
if (core_major == 1 && core_minor >= 0x49)
msm_host->updated_ddr_cfg = true;
+ ret = sdhci_msm_register_vreg(msm_host);
+ if (ret)
+ goto clk_disable;
+
/*
* Power on reset state may trigger power irq if previous status of
* PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
@@ -2116,6 +2314,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
MSM_MMC_AUTOSUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(&pdev->dev);
+ host->mmc_host_ops.start_signal_voltage_switch =
+ sdhci_msm_start_signal_voltage_switch;
host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
if (of_property_read_bool(node, "supports-cqe"))
ret = sdhci_msm_cqe_add_host(host, pdev);
@@ -2123,7 +2323,6 @@ static int sdhci_msm_probe(struct platform_device *pdev)
ret = sdhci_add_host(host);
if (ret)
goto pm_runtime_disable;
- sdhci_msm_set_regulator_caps(msm_host);
pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_put_autosuspend(&pdev->dev);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V1 1/3] dt-bindings: mmc: Supply max load for mmc supplies
From: Veerabhadrarao Badiganti @ 2020-05-15 11:18 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Veerabhadrarao Badiganti
In-Reply-To: <1589541535-8523-1-git-send-email-vbadigan@codeaurora.org>
Supply the max load needed for driving the mmc supplies.
Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
---
.../devicetree/bindings/mmc/mmc-controller.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
index acc9f10..9058b82 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml
@@ -290,6 +290,22 @@ properties:
description:
Supply for the bus IO line power
+ vmmc-max-load-microamp:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ - maximum: 1000000
+ description:
+ Maximum load for the card power.
+
+ vqmmc-max-load-microamp:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ - maximum: 1000000
+ description:
+ Maximum load for the bus IO line power.
+
mmc-pwrseq:
$ref: /schemas/types.yaml#/definitions/phandle
description:
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH V1 0/3] Internal voltage control for qcom SDHC
From: Veerabhadrarao Badiganti @ 2020-05-15 11:18 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, robh+dt
Cc: linux-mmc, linux-kernel, linux-arm-msm, devicetree,
Veerabhadrarao Badiganti
On qcom SD host controllers voltage switching be done after the HW
is ready for it. The HW informs its readiness through power irq.
The voltage switching should happen only then.
So added support to register voltage regulators from the msm driver
and use them.
This patchset was posted long back but not actively pursued
https://lore.kernel.org/linux-arm-msm/1539004739-32060-1-git-send-email-vbadigan@codeaurora.org/
So posting it as fresh patchset.
Veerabhadrarao Badiganti (1):
dt-bindings: mmc: Supply max load for mmc supplies
Vijay Viswanath (2):
mmc: sdhci-msm: Use internal voltage control
mmc: sdhci: Allow platform controlled voltage switching
.../devicetree/bindings/mmc/mmc-controller.yaml | 16 ++
drivers/mmc/host/sdhci-msm.c | 215 ++++++++++++++++++++-
drivers/mmc/host/sdhci.c | 32 +--
drivers/mmc/host/sdhci.h | 1 +
4 files changed, 243 insertions(+), 21 deletions(-)
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v2 1/7] usb: gadget: udc: atmel: use of_find_matching_node_and_match
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Claudiu Beznea
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Claudiu Beznea <claudiu.beznea@microchip.com>
Instead of trying to match every possible compatible use
of_find_matching_node_and_match() and pass the compatible array.
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 22200341c8ec..2b154085dc6a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2052,6 +2052,13 @@ static const struct of_device_id atmel_udc_dt_ids[] = {
MODULE_DEVICE_TABLE(of, atmel_udc_dt_ids);
+static const struct of_device_id atmel_pmc_dt_ids[] = {
+ { .compatible = "atmel,at91sam9g45-pmc" },
+ { .compatible = "atmel,at91sam9rl-pmc" },
+ { .compatible = "atmel,at91sam9x5-pmc" },
+ { /* sentinel */ }
+};
+
static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
struct usba_udc *udc)
{
@@ -2067,13 +2074,17 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
return ERR_PTR(-EINVAL);
udc->errata = match->data;
- udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9g45-pmc");
- if (IS_ERR(udc->pmc))
- udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9rl-pmc");
- if (IS_ERR(udc->pmc))
- udc->pmc = syscon_regmap_lookup_by_compatible("atmel,at91sam9x5-pmc");
- if (udc->errata && IS_ERR(udc->pmc))
- return ERR_CAST(udc->pmc);
+ if (udc->errata) {
+ pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
+ NULL);
+ if (!pp)
+ return ERR_PTR(-ENODEV);
+
+ udc->pmc = syscon_node_to_regmap(pp);
+ of_node_put(pp);
+ if (IS_ERR(udc->pmc))
+ return ERR_CAST(udc->pmc);
+ }
udc->num_ep = 0;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/7] dt-bindings: usb: atmel: Update DT bindings documentation for sam9x60
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Cristian Birsan <cristian.birsan@microchip.com>
Add sam9x60 binding.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
Documentation/devicetree/bindings/usb/atmel-usb.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/usb/atmel-usb.txt b/Documentation/devicetree/bindings/usb/atmel-usb.txt
index 44e80153b148..bae2b928a014 100644
--- a/Documentation/devicetree/bindings/usb/atmel-usb.txt
+++ b/Documentation/devicetree/bindings/usb/atmel-usb.txt
@@ -82,6 +82,7 @@ Required properties:
"atmel,at91sam9rl-udc"
"atmel,at91sam9g45-udc"
"atmel,sama5d3-udc"
+ "microchip,sam9x60-udc"
- reg: Address and length of the register set for the device
- interrupts: Should contain usba interrupt
- clocks: Should reference the peripheral and host clocks
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/7] usb: gadget: udc: atmel: simplify endpoint allocation
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Cristian Birsan <cristian.birsan@microchip.com>
Simplify the endpoint allocation and cleanup the code.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 21 ++++++++-------------
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 -
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 2b154085dc6a..beb7246935a8 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1097,7 +1097,6 @@ static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
ep->ept_cfg |= USBA_BF(BK_NUMBER, ep->nr_banks);
- ep->udc->configured_ep++;
}
return _ep;
@@ -1790,7 +1789,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (status & USBA_END_OF_RESET) {
struct usba_ep *ep0, *ep;
- int i, n;
+ int i;
usba_writel(udc, INT_CLR,
USBA_END_OF_RESET|USBA_END_OF_RESUME
@@ -1838,13 +1837,14 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
"ODD: EP0 configuration is invalid!\n");
/* Preallocate other endpoints */
- n = fifo_mode ? udc->num_ep : udc->configured_ep;
- for (i = 1; i < n; i++) {
+ for (i = 1; i < udc->num_ep; i++) {
ep = &udc->usba_ep[i];
- usba_ep_writel(ep, CFG, ep->ept_cfg);
- if (!(usba_ep_readl(ep, CFG) & USBA_EPT_MAPPED))
- dev_err(&udc->pdev->dev,
- "ODD: EP%d configuration is invalid!\n", i);
+ if (ep->ep.claimed) {
+ usba_ep_writel(ep, CFG, ep->ept_cfg);
+ if (!(usba_ep_readl(ep, CFG) & USBA_EPT_MAPPED))
+ dev_err(&udc->pdev->dev,
+ "ODD: EP%d configuration is invalid!\n", i);
+ }
}
}
@@ -2011,10 +2011,6 @@ static int atmel_usba_stop(struct usb_gadget *gadget)
if (udc->vbus_pin)
disable_irq(gpiod_to_irq(udc->vbus_pin));
- if (fifo_mode == 0)
- udc->configured_ep = 1;
-
- udc->suspended = false;
usba_stop(udc);
udc->driver = NULL;
@@ -2095,7 +2091,6 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
pp = NULL;
while ((pp = of_get_next_child(np, pp)))
udc->num_ep++;
- udc->configured_ep = 1;
} else {
udc->num_ep = usba_config_fifo_table(udc);
}
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h b/drivers/usb/gadget/udc/atmel_usba_udc.h
index a0225e4543d4..8de79356d31d 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -324,7 +324,6 @@ struct usba_udc {
int irq;
struct gpio_desc *vbus_pin;
int num_ep;
- int configured_ep;
struct usba_fifo_cfg *fifo_cfg;
struct clk *pclk;
struct clk *hclk;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 4/7] usb: gadget: udc: atmel: use 1 bank endpoints for control transfers
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Cristian Birsan <cristian.birsan@microchip.com>
Use 1 bank endpoints for control transfers
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index beb7246935a8..a73b0e78a357 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1061,6 +1061,7 @@ static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
switch (usb_endpoint_type(desc)) {
case USB_ENDPOINT_XFER_CONTROL:
+ ep->nr_banks = 1;
break;
case USB_ENDPOINT_XFER_ISOC:
--
2.17.1
^ permalink raw reply related
* [PATCH v2 6/7] usb: gadget: udc: atmel: update endpoint allocation for sam9x60
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Cristian Birsan <cristian.birsan@microchip.com>
The DPRAM memory from the USB High Speed Device Port (UDPHS) hardware
block was increased. This patch updates the endpoint allocation for sam9x60
to take advantage of this larger memory. At the same time the
constraint to allocate the endpoints in order was lifted. To handle old
and new hardware in the same driver the capabilities (caps) structure
was extended.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 22 ++++++++++++++++++----
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 +
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 2b1a0b6df0fe..ecd0fa9823bb 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1066,12 +1066,14 @@ static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
case USB_ENDPOINT_XFER_ISOC:
ep->fifo_size = 1024;
- ep->nr_banks = 2;
+ if (ep->udc->caps->ep_prealloc)
+ ep->nr_banks = 2;
break;
case USB_ENDPOINT_XFER_BULK:
ep->fifo_size = 512;
- ep->nr_banks = 1;
+ if (ep->udc->caps->ep_prealloc)
+ ep->nr_banks = 1;
break;
case USB_ENDPOINT_XFER_INT:
@@ -1081,7 +1083,8 @@ static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
else
ep->fifo_size =
roundup_pow_of_two(le16_to_cpu(desc->wMaxPacketSize));
- ep->nr_banks = 1;
+ if (ep->udc->caps->ep_prealloc)
+ ep->nr_banks = 1;
break;
}
@@ -2034,16 +2037,27 @@ static void at91sam9g45_pulse_bias(struct usba_udc *udc)
static const struct usba_udc_caps at91sam9rl_caps = {
.toggle_bias = at91sam9rl_toggle_bias,
+ .ep_prealloc = true,
};
static const struct usba_udc_caps at91sam9g45_caps = {
.pulse_bias = at91sam9g45_pulse_bias,
+ .ep_prealloc = true,
+};
+
+static const struct usba_udc_caps sama5d3_caps = {
+ .ep_prealloc = true,
+};
+
+static const struct usba_udc_caps at91sam9x60_caps = {
+ .ep_prealloc = false,
};
static const struct of_device_id atmel_udc_dt_ids[] = {
{ .compatible = "atmel,at91sam9rl-udc", .data = &at91sam9rl_caps },
{ .compatible = "atmel,at91sam9g45-udc", .data = &at91sam9g45_caps },
- { .compatible = "atmel,sama5d3-udc" },
+ { .compatible = "atmel,sama5d3-udc", .data = &sama5d3_caps },
+ { .compatible = "microchip,sam9x60-udc", .data = &at91sam9x60_caps },
{ /* sentinel */ }
};
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 1a0f77bf8d4f..f9239e200e7a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -305,6 +305,7 @@ struct usba_request {
struct usba_udc_caps {
void (*toggle_bias)(struct usba_udc *udc, int is_on);
void (*pulse_bias)(struct usba_udc *udc);
+ bool ep_prealloc;
};
struct usba_udc {
--
2.17.1
^ permalink raw reply related
* [PATCH v2 5/7] usb: gadget: udc: atmel: rename errata into caps
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Cristian Birsan <cristian.birsan@microchip.com>
Rename errata structure into capabilities (caps). It will be used to add
capabilities for new SoCs. Get the pointer to PMC only for the SoCs that
need it to perform toggle_bias or pulse_bias.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 20 ++++++++++----------
drivers/usb/gadget/udc/atmel_usba_udc.h | 4 ++--
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index a73b0e78a357..2b1a0b6df0fe 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -389,8 +389,8 @@ static int vbus_is_present(struct usba_udc *udc)
static void toggle_bias(struct usba_udc *udc, int is_on)
{
- if (udc->errata && udc->errata->toggle_bias)
- udc->errata->toggle_bias(udc, is_on);
+ if (udc->caps && udc->caps->toggle_bias)
+ udc->caps->toggle_bias(udc, is_on);
}
static void generate_bias_pulse(struct usba_udc *udc)
@@ -398,8 +398,8 @@ static void generate_bias_pulse(struct usba_udc *udc)
if (!udc->bias_pulse_needed)
return;
- if (udc->errata && udc->errata->pulse_bias)
- udc->errata->pulse_bias(udc);
+ if (udc->caps && udc->caps->pulse_bias)
+ udc->caps->pulse_bias(udc);
udc->bias_pulse_needed = false;
}
@@ -2032,17 +2032,17 @@ static void at91sam9g45_pulse_bias(struct usba_udc *udc)
AT91_PMC_BIASEN);
}
-static const struct usba_udc_errata at91sam9rl_errata = {
+static const struct usba_udc_caps at91sam9rl_caps = {
.toggle_bias = at91sam9rl_toggle_bias,
};
-static const struct usba_udc_errata at91sam9g45_errata = {
+static const struct usba_udc_caps at91sam9g45_caps = {
.pulse_bias = at91sam9g45_pulse_bias,
};
static const struct of_device_id atmel_udc_dt_ids[] = {
- { .compatible = "atmel,at91sam9rl-udc", .data = &at91sam9rl_errata },
- { .compatible = "atmel,at91sam9g45-udc", .data = &at91sam9g45_errata },
+ { .compatible = "atmel,at91sam9rl-udc", .data = &at91sam9rl_caps },
+ { .compatible = "atmel,at91sam9g45-udc", .data = &at91sam9g45_caps },
{ .compatible = "atmel,sama5d3-udc" },
{ /* sentinel */ }
};
@@ -2070,8 +2070,8 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
if (!match)
return ERR_PTR(-EINVAL);
- udc->errata = match->data;
- if (udc->errata) {
+ udc->caps = match->data;
+ if (udc->caps && (udc->caps->pulse_bias || udc->caps->toggle_bias)) {
pp = of_find_matching_node_and_match(NULL, atmel_pmc_dt_ids,
NULL);
if (!pp)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 8de79356d31d..1a0f77bf8d4f 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -302,7 +302,7 @@ struct usba_request {
unsigned int mapped:1;
};
-struct usba_udc_errata {
+struct usba_udc_caps {
void (*toggle_bias)(struct usba_udc *udc, int is_on);
void (*pulse_bias)(struct usba_udc *udc);
};
@@ -320,7 +320,7 @@ struct usba_udc {
struct usb_gadget gadget;
struct usb_gadget_driver *driver;
struct platform_device *pdev;
- const struct usba_udc_errata *errata;
+ const struct usba_udc_caps *caps;
int irq;
struct gpio_desc *vbus_pin;
int num_ep;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 7/7] ARM: dts: at91: sam9x60ek: enable usb device
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
In-Reply-To: <20200515111631.31210-1-cristian.birsan@microchip.com>
From: Cristian Birsan <cristian.birsan@microchip.com>
Enable usb device for sam9x60ek board.
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
arch/arm/boot/dts/at91-sam9x60ek.dts | 13 +++++
arch/arm/boot/dts/sam9x60.dtsi | 74 ++++++++++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
index b484745bf2d4..325d0fc8674f 100644
--- a/arch/arm/boot/dts/at91-sam9x60ek.dts
+++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
@@ -547,6 +547,12 @@
atmel,pins = <AT91_PIOD 18 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
};
};
+
+ usb0 {
+ pinctrl_usba_vbus: usba_vbus {
+ atmel,pins = <AT91_PIOB 16 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+ };
+ };
}; /* pinctrl */
&pmc {
@@ -634,6 +640,13 @@
};
};
+&usb0 {
+ atmel,vbus-gpio = <&pioB 16 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usba_vbus>;
+ status = "okay";
+};
+
&usb1 {
num-ports = <3>;
atmel,vbus-gpio = <0
diff --git a/arch/arm/boot/dts/sam9x60.dtsi b/arch/arm/boot/dts/sam9x60.dtsi
index 6763423d64b8..5cd2b9054762 100644
--- a/arch/arm/boot/dts/sam9x60.dtsi
+++ b/arch/arm/boot/dts/sam9x60.dtsi
@@ -69,6 +69,80 @@
#size-cells = <1>;
ranges;
+ usb0: gadget@500000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "microchip,sam9x60-udc";
+ reg = <0x00500000 0x100000
+ 0xf803c000 0x400>;
+ interrupts = <23 IRQ_TYPE_LEVEL_HIGH 2>;
+ clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_CORE PMC_UTMI>;
+ clock-names = "pclk", "hclk";
+ assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>;
+ assigned-clock-rates = <480000000>;
+ status = "disabled";
+
+ ep@0 {
+ reg = <0>;
+ atmel,fifo-size = <64>;
+ atmel,nb-banks = <1>;
+ };
+
+ ep@1 {
+ reg = <1>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <2>;
+ atmel,can-dma;
+ };
+
+ ep@2 {
+ reg = <2>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <2>;
+ atmel,can-dma;
+ };
+
+ ep@3 {
+ reg = <3>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <3>;
+ atmel,can-dma;
+ atmel,can-isoc;
+ };
+
+ ep@4 {
+ reg = <4>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <3>;
+ atmel,can-dma;
+ atmel,can-isoc;
+ };
+
+ ep@5 {
+ reg = <5>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <3>;
+ atmel,can-dma;
+ atmel,can-isoc;
+ };
+
+ ep@6 {
+ reg = <6>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <3>;
+ atmel,can-dma;
+ atmel,can-isoc;
+ };
+
+ ep@7 {
+ reg = <7>;
+ atmel,fifo-size = <1024>;
+ atmel,nb-banks = <3>;
+ atmel,can-dma;
+ atmel,can-isoc;
+ };
+ };
+
usb1: ohci@600000 {
compatible = "atmel,at91rm9200-ohci", "usb-ohci";
reg = <0x00600000 0x100000>;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/7] usb: gadget: udc: atmel: add usb device support for SAM9x60 SoC
From: cristian.birsan @ 2020-05-15 11:16 UTC (permalink / raw)
To: balbi, gregkh, nicolas.ferre, alexandre.belloni,
ludovic.desroches, robh+dt, mark.rutland, linux-arm-kernel,
linux-usb, devicetree, linux-kernel
Cc: Cristian Birsan
From: Cristian Birsan <cristian.birsan@microchip.com>
This patch set adds usb device support for SAM9x60 SoC.
The DPRAM memory for the USB High Speed Device Port (UDPHS) hardware
block was increased and the allocation method is changed. This patch
series simplifies the endpoint allocation scheme to acomodate this SoC
and the old ones.
Changes in v2:
- drop the patch that adds reference to pmc for sam9x60
- use dt-bindings: usb prefix
- enable usb device in device tree
Claudiu Beznea (1):
usb: gadget: udc: atmel: use of_find_matching_node_and_match
Cristian Birsan (6):
dt-bindings: usb: atmel: Update DT bindings documentation for sam9x60
usb: gadget: udc: atmel: simplify endpoint allocation
usb: gadget: udc: atmel: use 1 bank endpoints for control transfers
usb: gadget: udc: atmel: rename errata into caps
usb: gadget: udc: atmel: update endpoint allocation for sam9x60
ARM: dts: at91: sam9x60ek: enable usb device
.../devicetree/bindings/usb/atmel-usb.txt | 1 +
arch/arm/boot/dts/at91-sam9x60ek.dts | 13 +++
arch/arm/boot/dts/sam9x60.dtsi | 74 ++++++++++++++++
drivers/usb/gadget/udc/atmel_usba_udc.c | 87 ++++++++++++-------
drivers/usb/gadget/udc/atmel_usba_udc.h | 6 +-
5 files changed, 145 insertions(+), 36 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Serge Semin @ 2020-05-15 11:11 UTC (permalink / raw)
To: Vinod Koul
Cc: Serge Semin, Andy Shevchenko, Viresh Kumar, Rob Herring,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200515105658.GR333670@vkoul-mobl>
On Fri, May 15, 2020 at 04:26:58PM +0530, Vinod Koul wrote:
> On 15-05-20, 13:51, Andy Shevchenko wrote:
> > On Fri, May 15, 2020 at 11:39:11AM +0530, Vinod Koul wrote:
> > > On 12-05-20, 15:38, Andy Shevchenko wrote:
> > > > On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > > > > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > > > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
> >
> > ...
> >
> > > > I leave it to Rob and Vinod.
> > > > It won't break our case, so, feel free with your approach.
> > >
> > > I agree the DT is about describing the hardware and looks like value of
> > > 1 is not allowed. If allowed it should be added..
> >
> > It's allowed at *run time*, it's illegal in *pre-silicon stage* when
> > synthesizing the IP.
>
> Then it should be added ..
Vinod, max-burst-len is "MAXimum" burst length not "run-time or current or any
other" burst length. It's a constant defined at the IP-core synthesis stage and
according to the Data Book, MAX burst length can't be 1. The allowed values are
exactly as I described in the binding [4, 8, 16, 32, ...]. MAX burst length
defines the upper limit of the run-time burst length. So setting it to 1 isn't
about describing a hardware, but using DT for the software convenience.
-Sergey
>
> --
> ~Vinod
^ permalink raw reply
* Re: [PATCH v2 0/2] ASoC: fsl_esai: Add support for imx8qm
From: Mark Brown @ 2020-05-15 11:10 UTC (permalink / raw)
To: nicoleotsuka, robh+dt, lgirdwood, tiwai, alsa-devel, timur,
Xiubo.Lee, Shengjiu Wang, perex, festevam, devicetree
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <cover.1589537601.git.shengjiu.wang@nxp.com>
On Fri, 15 May 2020 18:10:49 +0800, Shengjiu Wang wrote:
> Add support for imx8qm.
>
> Shengjiu Wang (2):
> ASoC: fsl_esai: introduce SoC specific data
> ASoC: fsl_esai: Add new compatible string for imx8qm
>
> Changes in v2
> - drop the 0002 patch in v1, the dma relate limitation should
> be done in dma driver, or define a new DMA API for it.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.8
Thanks!
[1/2] ASoC: fsl_esai: introduce SoC specific data
commit: 6878e75204e1d0420fd8130bad33f88053ba44de
[2/2] ASoC: fsl_esai: Add new compatible string for imx8qm
commit: d59628b310a77e616ce2e5857e6ede5bf96c6784
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: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Srinivas Kandagatla @ 2020-05-15 11:09 UTC (permalink / raw)
To: Ravi Kumar Bokka (Temp), Rob Herring
Cc: linux-kernel, devicetree, rnayak, saiprakash.ranjan, dhavalp,
mturney, sparate, c_rbokka, mkurumel, dianders
In-Reply-To: <14e1fa51-066c-6e1b-01a4-2103612de9e9@codeaurora.org>
On 14/05/2020 13:26, Ravi Kumar Bokka (Temp) wrote:
> Hi Srinivas,
> Thanks for your feedback by giving review comments. Please find my
> inline comments.
>
>
> Regards,
> Ravi Kumar.B
>
> On 5/13/2020 6:50 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 12/05/2020 19:17, Ravi Kumar Bokka wrote:
>>> This patch adds new driver for QTI qfprom-efuse controller. This
>>> driver can
>>> access the raw qfprom regions for fuse blowing.
>>
>> QTI?
>
> guidance I have received from internal Legal/LOST team is that the QCOM
> prefix needs to be changed to QTI everywhere it is used
>
I dont mind this in comments or patch subject line but the valid vendor
prefix for Qualcomm is "qcom".
>>
>>>
>>> The current existed qfprom driver is only supports for cpufreq,
>>> thermal sensors
>>> drivers by read out calibration data, speed bins..etc which is stored
>>> by qfprom efuses.
>>
>> Can you explain bit more about this QFPROM instance, Is this QFPROM
>> part of secure controller address space?
>> Is this closely tied to SoC or Secure controller version?
>>
>> Any reason why this can not be integrated into qfprom driver with
>> specific compatible.
>>
>
> QFPROM driver communicates with sec_controller address space however
> scope and functionalities of this driver is different and not limited as
> existing qfprom fuse Read-Only driver for specific “fuse buckets’ like
> cpufreq, thermal sensors etc. QFPROM fuse write driver in this patch
> requires specific sequence to write/blow fuses unlike other driver.
> Scope/functionalities are different and this is separate driver.
>
This is another variant of qfprom, so please add this support in the
existing qfprom driver, you could deal with the differences using
specific compatible within the driver.
Doug already covered most of the comments, consider them as mandatory
requirements for upstreaming!
--srini
>>>
>>> Signed-off-by: Ravi Kumar Bokka <rbokka@codeaurora.org>
>>> ---
>>> drivers/nvmem/Kconfig | 10 +
>>> drivers/nvmem/Makefile | 2 +
>>> drivers/nvmem/qfprom-efuse.c | 476
>>> +++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 488 insertions(+)
>>> create mode 100644 drivers/nvmem/qfprom-efuse.c
>>>
>> ...
>>
>>> diff --git a/drivers/nvmem/qfprom-efuse.c b/drivers/nvmem/qfprom-efuse.c
>>> new file mode 100644
>>> index 0000000..2e3c275
>>> --- /dev/null
>>> +++ b/drivers/nvmem/qfprom-efuse.c
>>> @@ -0,0 +1,476 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/device.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mod_devicetable.h>
>>> +#include <linux/nvmem-provider.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regulator/consumer.h>
>>> +
>>> +#define QFPROM_BLOW_STATUS_BUSY 0x1
>>> +#define QFPROM_BLOW_STATUS_READY 0x0
>>> +
>>> +/* Blow timer clock frequency in Mhz for 10nm LPe technology */
>>> +#define QFPROM_BLOW_TIMER_OFFSET 0x03c
>>> +#define QFPROM_BLOW_TIMER_RESET_VALUE 0x0
>>> +
>>> +/* Amount of time required to hold charge to blow fuse in
>>> micro-seconds */
>>> +#define QFPROM_FUSE_BLOW_POLL_PERIOD 100
>>> +#define QFPROM_BLOW_STATUS_OFFSET 0x048
>>> +
>>> +#define QFPROM_ACCEL_OFFSET 0x044
>>> +
>>> +/**
>>> + * struct qfprom_efuse_platform_data - structure holding qfprom-efuse
>>> + * platform data
>>> + *
>>> + * @name: qfprom-efuse compatible name
>>
>> ??
>
> Thanks for your feedback. I will address this change
>
>>> + * @fuse_blow_time_in_us: Should contain the wait time when doing
>>> the fuse blow
>>> + * @accel_value: Should contain qfprom accel value
>>> + * @accel_reset_value: The reset value of qfprom accel value
>>> + * @qfprom_blow_timer_value: The timer value of qfprom when doing
>>> efuse blow
>>> + * @qfprom_blow_reset_freq: The frequency required to set when fuse
>>> blowing
>>> + * is done
>>> + * @qfprom_blow_set_freq: The frequency required to set when we
>>> start the
>>> + * fuse blowing
>>> + * @qfprom_max_vol: max voltage required to set fuse blow
>>> + * @qfprom_min_vol: min voltage required to set fuse blow
>>
>> How specific are these values per SoC?
>>
>
> This voltage level may change based on SoC and/or fuse-hardware
> technology, it would change for SoC with different technology, hence we
> have kept it in SOC specific settings.
>
>>
>>> + */
>>> +struct qfprom_efuse_platform_data {
>>> + const char *name;
>>> + u8 fuse_blow_time_in_us;
>>> + u32 accel_value;
>>> + u32 accel_reset_value;
>>> + u32 qfprom_blow_timer_value;
>>> + u32 qfprom_blow_reset_freq;
>>> + u32 qfprom_blow_set_freq;
>>> + u32 qfprom_max_vol;
>>> + u32 qfprom_min_vol;
>>> +};
>>> +
>>> +/**
>>> + * struct qfprom_efuse_priv - structure holding qfprom-efuse attributes
>>> + *
>>> + * @qfpbase: iomapped memory space for qfprom base
>>> + * @qfpraw: iomapped memory space for qfprom raw fuse region
>>> + * @qfpmap: iomapped memory space for qfprom fuse blow timer
>>> +
>>> + * @dev: qfprom device structure
>>> + * @secclk: clock supply
>>> + * @vcc: regulator supply
>>> +
>>> + * @qfpraw_start: qfprom raw fuse start region
>>> + * @qfpraw_end: qfprom raw fuse end region
>>> + * @qfprom_efuse_platform_data: qfprom platform data
>>> + */
>>> +struct qfprom_efuse_priv {
>>> + void __iomem *qfpbase;
>>> + void __iomem *qfpraw;
>>> + void __iomem *qfpmap;
>>
>> Why are these memory regions split? Can't you just have complete
>> qfprom area and add fixed offset for qfpraw within the driver?
>>
>
> Thanks for your feedback. I will address this change.
> I have separated this memory regions because to identify raw fuse
> regions separately and compare these raw fuse regions from the user
> given input.
>
>>> + struct device *dev;
>>> + struct clk *secclk;
>>> + struct regulator *vcc;
>>> + resource_size_t qfpraw_start;
>>> + resource_size_t qfpraw_end;
>> Why do we need to check this range? as long as we set the nvmem_config
>> with correct range then you should not need this check.
>>
>
> There is no harm in this explicit check in QFPROM-fuse driver and based
> on internal review with our security team, this check is important to
> avoid dependency on other upper layer.
>
>
>>
>>> + struct qfprom_efuse_platform_data efuse;
>> A pointer here should be good enough?
>>> +};
>>> +
>>
>
> Thanks for your feedback. I will address this change
>
>> ...
>>
>>> +/*
>>> + * sets the value of the blow timer, accel register and the clock
>>> + * and voltage settings
>>> + */
>>> +static int qfprom_enable_fuse_blowing(const struct qfprom_efuse_priv
>>> *priv)
>>> +{
>>> + int ret;
>>> +
>>> + ret = qfprom_disable_fuse_blowing(priv);
>>> + if (ret) {
>>> + dev_err(priv->dev, "qfprom_disable_fuse_blowing()\n");
>>> + return ret;
>>> + }
>>
>> Why do we need to qfprom_disable_fuse_blowing() for every call to
>> enable it?
>>
>> Or are we missing some error handling in the caller?
>>
>
> We must disable/vote-off this QFPROM fuse power rail after blowing fuse,
> it is the safe and right approach as per hardware programming guide for
> fuse blowing process. Caller here is user space, can’t control
> fuse-power-rail or can’t be relied to follow the required process. There
> could also be unnecessary risk of leaving the vote/power-rail configured
> at specific level after blowing the fuse. As per hardware requirement,
> right after fuse blowing, we need to disable power rail.
>
>>> +
>>> + writel(priv->efuse.qfprom_blow_timer_value, priv->qfpmap +
>>> + QFPROM_BLOW_TIMER_OFFSET);
>>> + writel(priv->efuse.accel_value, priv->qfpmap +
>>> QFPROM_ACCEL_OFFSET);
>>> +
>>> + ret = qfprom_set_clock_settings(priv);
>>> + if (ret) {
>>> + dev_err(priv->dev, "qpfrom_set_clock_settings()\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = qfprom_set_voltage_settings(priv, priv->efuse.qfprom_min_vol,
>>> + priv->efuse.qfprom_max_vol);
>>> + if (ret) {
>>> + dev_err(priv->dev, "qfprom_set_voltage_settings()\n");
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>
>> <<
>>> +/*
>>> + * verifying to make sure address being written or read is from qfprom
>>> + * raw address range
>>> + */
>>> +bool addr_in_qfprom_range(const struct qfprom_efuse_priv *priv, u32
>>> reg,
>>> + size_t bytes)
>>> +{
>>> + if (((reg + bytes) > reg) && (reg >= priv->qfpraw_start) &&
>>> + ((reg + bytes) <= priv->qfpraw_end)) {
>>> + return 1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>> >>
>> Above function is totally redundant, nvmem core already has checks for
>> this.
>>
>
> There is no harm in this explicit check in QFPROM-fuse driver and based
> on internal review with our security team, this check is important to
> avoid dependency on other upper layer.
>
>>
>>
>>> +
>>> +/*
>>> + * API for reading from raw qfprom region
>>> + */
>>> +static int qfprom_efuse_reg_read(void *context, unsigned int reg,
>>> void *_val,
>>> + size_t bytes)
>>> +{
>>> + struct qfprom_efuse_priv *priv = context;
>>> + u32 *value = _val;
>>> + u32 align_check;
>>> + int i = 0, words = bytes / 4;
>>> +
>>> + dev_info(priv->dev,
>>> + "reading raw qfprom region offset: 0x%08x of size: %zd\n",
>>> + reg, bytes);
>>
>> In general there is lot of debug info across the code, do you really
>> need all this? Consider removing these!
>>
>
> Thanks for your feedback. I will address this change.
>
>>> +
>>> + if (bytes % 4 != 0x00) {
>>> + dev_err(priv->dev,
>>> + "Bytes: %zd to read should be word align\n",
>>> + bytes);
>>> + return -EINVAL;
>>> + }
>>
>> This word align check is also redundant once you set nvmem_config with
>> correct word_size.
>>
>
> I understand that there may be different approach to handle this. We
> have used this approach and tested this driver thoroughly. Unless there
> is technical limitation, changing this word_size would end up requiring
> re-writing write/read APIs and going through testing again, there is not
> much difference in either approach, we would like to keep this approach
> unless there is technical concern.
>
>>
>>> +
>>> + if (!addr_in_qfprom_range(priv, reg, bytes)) {
>>> + dev_err(priv->dev,
>>> + "Invalid qfprom raw region offset 0x%08x & bytes %zd\n",
>>> + reg, bytes);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + align_check = (reg & 0xF);
>>> +
>>> + if (((align_check & ~3) == align_check) && value != NULL)
>>> + while (words--)
>>> + *value++ = readl(priv->qfpbase + reg + (i++ * 4));
>>> +
>>> + else
>>> + dev_err(priv->dev,
>>> + "Invalid input parameter 0x%08x fuse blow address\n",
>>> + reg);
>>> +
>>> + return 0;
>>> +}
>> ...
>>
>>> +
>>> +static int qfprom_efuse_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct resource *qfpbase, *qfpraw, *qfpmap;
>>> + struct nvmem_device *nvmem;
>>> + struct nvmem_config *econfig;
>>> + struct qfprom_efuse_priv *priv;
>>> + const struct qfprom_efuse_platform_data *drvdata;
>>> + int ret;
>>> +
>>> + dev_info(&pdev->dev, "[%s]: Invoked\n", __func__);
>>> +
>>
>> too much debug!
>>
>
> Thanks for your feedback. I will address this change.
>
>>> + drvdata = of_device_get_match_data(&pdev->dev);
>>> + if (!drvdata)
>>> + return -EINVAL;
>> Unnecessary check as this driver will not be probed unless there is a
>> compatible match.
>>
>
> Thanks for your feedback. I will address this change.
>
>>
>>> +
>>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> +
>>> + priv->efuse.fuse_blow_time_in_us = drvdata->fuse_blow_time_in_us;
>>> + priv->efuse.accel_value = drvdata->accel_value;
>>> + priv->efuse.accel_reset_value = drvdata->accel_reset_value;
>>> + priv->efuse.qfprom_blow_timer_value =
>>> drvdata->qfprom_blow_timer_value;
>>> + priv->efuse.qfprom_blow_reset_freq =
>>> drvdata->qfprom_blow_reset_freq;
>>> + priv->efuse.qfprom_blow_set_freq = drvdata->qfprom_blow_set_freq;
>>> + priv->efuse.qfprom_max_vol = drvdata->qfprom_max_vol;
>>> + priv->efuse.qfprom_min_vol = drvdata->qfprom_min_vol;
>>> + priv->dev = dev;
>>> +
>>> + qfpbase = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +
>>> + priv->qfpbase = devm_ioremap_resource(dev, qfpbase);
>>> + if (IS_ERR(priv->qfpbase)) {
>>> + ret = PTR_ERR(priv->qfpbase);
>>> + goto err;
>>> + }
>>> +
>>> + qfpraw = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> +
>>> + priv->qfpraw = devm_ioremap_resource(dev, qfpraw);
>>> + if (IS_ERR(priv->qfpraw)) {
>>> + ret = PTR_ERR(priv->qfpraw);
>>> + goto err;
>>> + }
>>> +
>>> + priv->qfpraw_start = qfpraw->start - qfpbase->start;
>>> + priv->qfpraw_end = qfpraw->end - qfpbase->start;
>>> +
>>> + qfpmap = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>>> +
>>> + priv->qfpmap = devm_ioremap_resource(dev, qfpmap);
>>> + if (IS_ERR(priv->qfpmap)) {
>>> + ret = PTR_ERR(priv->qfpmap);
>>> + goto err;
>>> + }
>>> +
>>> + priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
>>
>> I see no reference to this regulator in dt bindings.
>
> This perameter kept in board specific file i.e., sc7180-idp.dts file
>
>>> + if (IS_ERR(priv->vcc)) {
>>> + ret = PTR_ERR(priv->vcc);
>>> + if (ret == -ENODEV)
>>> + ret = -EPROBE_DEFER;
>> Can you explain what is going on here?
>>
>
> As i took other drivers reference, i have kept this check.
>
>>> +
>>> + goto err;
>>> + }
>>> +
>>> + priv->secclk = devm_clk_get(dev, "secclk");
>>> + if (IS_ERR(priv->secclk)) {
>>> + ret = PTR_ERR(priv->secclk);
>>> + if (ret != -EPROBE_DEFER)
>>> + dev_err(dev, "secclk error getting : %d\n", ret);
>>> + goto err;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->secclk);
>>> + if (ret) {
>>> + dev_err(dev, "clk_prepare_enable() failed\n");
>>> + goto err;
>>> + }
>>> +
>>> + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
>>> + if (!econfig)
>> Why not disabling the clk here?
>>> + return -ENOMEM;
>>
>
> Thanks for your feedback. I will address this change.
>
>>> +
>>> + econfig->dev = dev;
>>> + econfig->name = "qfprom-efuse";
>>> + econfig->stride = 1;
>>> + econfig->word_size = 1;
>>> + econfig->reg_read = qfprom_efuse_reg_read;
>>> + econfig->reg_write = qfprom_efuse_reg_write;
>>> + econfig->size = resource_size(qfpraw);
>>> + econfig->priv = priv;
>>> +
>>> + nvmem = devm_nvmem_register(dev, econfig);
>>> +
>>> + return PTR_ERR_OR_ZERO(nvmem);
>> probably you should check the nvmem here before returning to disable
>> the clk properly.
>>
>
> Thanks for your feedback. I will address this change.
>
>>> +
>>> +err:
>>> + clk_disable_unprepare(priv->secclk);
>>> + return ret;
>>> +}
>>> +
>>> +static const struct qfprom_efuse_platform_data sc7180_qfp_efuse_data
>>> = {
>>> + .name = "sc7180-qfprom-efuse",
>> Redundant.
>>
>
> Thanks for your feedback. I will address this change.
>
>>> + .fuse_blow_time_in_us = 10,
>>> + .accel_value = 0xD10,
>>> + .accel_reset_value = 0x800,
>>> + .qfprom_blow_timer_value = 25,
>>> + .qfprom_blow_reset_freq = 19200000,
>>> + .qfprom_blow_set_freq = 4800000,
>>> + .qfprom_max_vol = 1904000,
>>> + .qfprom_min_vol = 1800000,
>>> +};
>>> +
>>> +static const struct of_device_id qfprom_efuse_of_match[] = {
>>> + {
>>> + .compatible = "qcom,sc7180-qfprom-efuse",
>>> + .data = &sc7180_qfp_efuse_data
>>> + },
>>> + {/* sentinel */},
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(of, qfprom_efuse_of_match);
>>> +
>>> +static struct platform_driver qfprom_efuse_driver = {
>>> + .probe = qfprom_efuse_probe,
>>> + .driver = {
>>> + .name = "sc7180-qfprom-efuse",
>>> + .of_match_table = qfprom_efuse_of_match,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(qfprom_efuse_driver);
>>> +MODULE_DESCRIPTION("QTI QFPROM Efuse driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>
^ permalink raw reply
* Re: [PATCH v2 5/6] dmaengine: dw: Introduce max burst length hw config
From: Andy Shevchenko @ 2020-05-15 11:02 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Vinod Koul, Viresh Kumar, Dan Williams,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200512194734.j5xvm3khijpp5tkh@mobilestation>
On Tue, May 12, 2020 at 10:47:34PM +0300, Serge Semin wrote:
> On Tue, May 12, 2020 at 10:12:08PM +0300, Andy Shevchenko wrote:
> > On Tue, May 12, 2020 at 05:08:20PM +0300, Serge Semin wrote:
> > > On Fri, May 08, 2020 at 02:41:53PM +0300, Andy Shevchenko wrote:
> > > > On Fri, May 08, 2020 at 01:53:03PM +0300, Serge Semin wrote:
> > > > > IP core of the DW DMA controller may be synthesized with different
> > > > > max burst length of the transfers per each channel. According to Synopsis
> > > > > having the fixed maximum burst transactions length may provide some
> > > > > performance gain. At the same time setting up the source and destination
> > > > > multi size exceeding the max burst length limitation may cause a serious
> > > > > problems. In our case the system just hangs up. In order to fix this
> > > > > lets introduce the max burst length platform config of the DW DMA
> > > > > controller device and don't let the DMA channels configuration code
> > > > > exceed the burst length hardware limitation. Depending on the IP core
> > > > > configuration the maximum value can vary from channel to channel.
> > > > > It can be detected either in runtime from the DWC parameter registers
> > > > > or from the dedicated dts property.
> > > >
> > > > I'm wondering what can be the scenario when your peripheral will ask something
> > > > which is not supported by DMA controller?
> > >
> > > I may misunderstood your statement, because seeing your activity around my
> > > patchsets including the SPI patchset and sometimes very helpful comments,
> > > this question answer seems too obvious to see you asking it.
> > >
> > > No need to go far for an example. See the DW APB SSI driver. Its DMA module
> > > specifies the burst length to be 16, while not all of ours channels supports it.
> > > Yes, originally it has been developed for the Intel Midfield SPI, but since I
> > > converted the driver into a generic code we can't use a fixed value. For instance
> > > in our hardware only two DMA channels of total 16 are capable of bursting up to
> > > 16 bytes (data items) at a time, the rest of them are limited with up to 4 bytes
> > > burst length. While there are two SPI interfaces, each of which need to have two
> > > DMA channels for communications. So I need four channels in total to allocate to
> > > provide the DMA capability for all interfaces. In order to set the SPI controller
> > > up with valid optimized parameters the max-burst-length is required. Otherwise we
> > > can end up with buffers overrun/underrun.
> >
> > Right, and we come to the question which channel better to be used by SPI and
> > the rest devices. Without specific filter function you can easily get into a
> > case of inverted optimizations, when SPI got channels with burst = 4, while
> > it's needed 16, and other hardware otherwise. Performance wise it's worse
> > scenario which we may avoid in the first place, right?
>
> If we start thinking like you said, we'll get stuck at a problem of which interfaces
> should get faster DMA channels and which one should be left with slowest. In general
> this task can't be solved, because without any application-specific requirement
> they all are equally valuable and deserve to have the best resources allocated.
> So we shouldn't assume that some interface is better or more valuable than
> another, therefore in generic DMA client code any filtering is redundant.
True, that's why I called it platform dependent quirks. You may do whatever you
want / need to preform on your hardware best you can. If it's okay for your
hardware to have this inverse optimization, than fine, generic DMA client
should really not care about it.
> > > > Peripheral needs to supply a lot of configuration parameters specific to the
> > > > DMA controller in use (that's why we have struct dw_dma_slave).
> > > > So, seems to me the feasible approach is supply correct data in the first place.
> > >
> > > How to supply a valid data if clients don't know the DMA controller limitations
> > > in general?
> >
> > This is a good question. DMA controllers are quite different and having unified
> > capabilities structure for all is almost impossible task to fulfil. That's why
> > custom filter function(s) can help here. Based on compatible string you can
> > implement whatever customized quirks like two functions, for example, to try 16
> > burst size first and fallback to 4 if none was previously found.
>
> Right. As I said in the previous email it's up to the corresponding platforms to
> decide the criteria of the filtering including the max-burst length value.
Correct!
> Even though the DW DMA channels resources aren't uniform on Baikal-T1 SoC I also
> won't do the filter-based channel allocation, because I can't predict the SoC
> application. Some of them may be used on a platform with active SPI interface
> utilization, some with specific requirements to UARTs and so on.
It's your choice as platform maintainer.
> > > > If you have specific channels to acquire then you probably need to provide a
> > > > custom xlate / filter functions. Because above seems a bit hackish workaround
> > > > of dynamic channel allocation mechanism.
> > >
> > > No, I don't have a specific channel to acquire and in general you may use any
> > > returned from the DMA subsystem (though some platforms may need a dedicated
> > > channels to use, in this case xlate / filter is required). In our SoC any DW DMAC
> > > channel can be used for any DMA-capable peripherals like SPI, I2C, UART. But the
> > > their DMA settings must properly and optimally configured. It can be only done
> > > if you know the DMA controller parameters like max burst length, max block-size,
> > > etc.
> > >
> > > So no. The change proposed by this patch isn't workaround, but a useful feature,
> > > moreover expected to be supported by the generic DMA subsystem.
> >
> > See above.
> >
> > > > But let's see what we can do better. Since maximum is defined on the slave side
> > > > device, it probably needs to define minimum as well, otherwise it's possible
> > > > that some hardware can't cope underrun bursts.
> > >
> > > There is no need to define minimum if such limit doesn't exists except a
> > > natural 1. Moreover it doesn't exist for all DMA controllers seeing noone has
> > > added such capability into the generic DMA subsystem so far.
> >
> > There is a contract between provider and consumer about DMA resource. That's
> > why both sides should participate in fulfilling it. Theoretically it may be a
> > hardware that doesn't support minimum burst available in DMA by a reason. For
> > such we would need minimum to be provided as well.
>
> I don't think 'theoretical' consideration counts when implementing something in
> kernel. That 'theoretical' may never happen, but you'll end up supporting a
> dummy functionality. Practicality is what kernel developers normally place
> before anything else.
The point here is to avoid half-baked solutions.
I'm not against max-burst logic on top of the existing interface, but would be
better if we allow the range, in this case it will work for any DMA controller
(as be part of DMA engine family).
I guess we need summarize this very long discussion and settle the next steps.
(if you can provide in short form anybody can read in 1 minute it would be
nice, I already forgot tons of paragraphs you sent here, esp. taking into
account tons of paragraphs in the other Baikal related series)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Vinod Koul @ 2020-05-15 10:56 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Serge Semin, Viresh Kumar, Rob Herring,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200515105137.GK185537@smile.fi.intel.com>
On 15-05-20, 13:51, Andy Shevchenko wrote:
> On Fri, May 15, 2020 at 11:39:11AM +0530, Vinod Koul wrote:
> > On 12-05-20, 15:38, Andy Shevchenko wrote:
> > > On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > > > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
>
> ...
>
> > > I leave it to Rob and Vinod.
> > > It won't break our case, so, feel free with your approach.
> >
> > I agree the DT is about describing the hardware and looks like value of
> > 1 is not allowed. If allowed it should be added..
>
> It's allowed at *run time*, it's illegal in *pre-silicon stage* when
> synthesizing the IP.
Then it should be added ..
--
~Vinod
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox