* Re: [PATCH v7 2/2] dmaengine: Loongson1: Add Loongson-1 APB DMA driver
From: Keguang Zhang @ 2024-04-02 10:50 UTC (permalink / raw)
To: Huacai Chen
Cc: Vinod Koul, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-mips, dmaengine, devicetree, linux-kernel
In-Reply-To: <CAAhV-H5uLcfaNYb7GAF17ruhJ02Wv71VZYEnxM_a642cuYaSBw@mail.gmail.com>
On Tue, Apr 2, 2024 at 5:04 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Tue, Apr 2, 2024 at 9:56 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> >
> > On Mon, Apr 1, 2024 at 9:24 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > On Mon, Apr 1, 2024 at 7:10 PM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 1, 2024 at 5:06 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > >
> > > > > On Mon, Apr 1, 2024 at 10:45 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > > > > >
> > > > > > Hi Huacai,
> > > > > >
> > > > > > On Sat, Mar 30, 2024 at 9:59 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > > >
> > > > > > > Hi, Keguang,
> > > > > > >
> > > > > > > On Fri, Mar 29, 2024 at 7:28 PM Keguang Zhang via B4 Relay
> > > > > > > <devnull+keguang.zhang.gmail.com@kernel.org> wrote:
> > > > > > > >
> > > > > > > > From: Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > >
> > > > > > > > This patch adds APB DMA driver for Loongson-1 SoCs.
> > > > > > > >
> > > > > > > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > > ---
> > > > > > > > Changes in v7:
> > > > > > > > - Change the comptible to 'loongson,ls1*-apbdma'
> > > > > > > > - Update Kconfig and Makefile accordingly
> > > > > > > > - Rename the file to loongson1-apb-dma.c to keep the consistency
> > > > > > > >
> > > > > > > > Changes in v6:
> > > > > > > > - Implement .device_prep_dma_cyclic for Loongson1 audio driver,
> > > > > > > > - as well as .device_pause and .device_resume.
> > > > > > > > - Set the limitation LS1X_DMA_MAX_DESC and put all descriptors
> > > > > > > > - into one page to save memory
> > > > > > > > - Move dma_pool_zalloc() into ls1x_dma_alloc_desc()
> > > > > > > > - Drop dma_slave_config structure
> > > > > > > > - Use .remove_new instead of .remove
> > > > > > > > - Use KBUILD_MODNAME for the driver name
> > > > > > > > - Improve the debug information
> > > > > > > >
> > > > > > > > Changes in v5:
> > > > > > > > - Add DT support
> > > > > > > > - Use DT data instead of platform data
> > > > > > > > - Use chan_id of struct dma_chan instead of own id
> > > > > > > > - Use of_dma_xlate_by_chan_id() instead of ls1x_dma_filter()
> > > > > > > > - Update the author information to my official name
> > > > > > > >
> > > > > > > > Changes in v4:
> > > > > > > > - Use dma_slave_map to find the proper channel.
> > > > > > > > - Explicitly call devm_request_irq() and tasklet_kill().
> > > > > > > > - Fix namespace issue.
> > > > > > > > - Some minor fixes and cleanups.
> > > > > > > >
> > > > > > > > Changes in v3:
> > > > > > > > - Rename ls1x_dma_filter_fn to ls1x_dma_filter.
> > > > > > > >
> > > > > > > > Changes in v2:
> > > > > > > > - Change the config from 'DMA_LOONGSON1' to 'LOONGSON1_DMA',
> > > > > > > > - and rearrange it in alphabetical order in Kconfig and Makefile.
> > > > > > > > - Fix comment style.
> > > > > > > > ---
> > > > > > > > drivers/dma/Kconfig | 9 +
> > > > > > > > drivers/dma/Makefile | 1 +
> > > > > > > > drivers/dma/loongson1-apb-dma.c | 665 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > > 3 files changed, 675 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > > > > > > > index 002a5ec80620..f7b06c4cdf3f 100644
> > > > > > > > --- a/drivers/dma/Kconfig
> > > > > > > > +++ b/drivers/dma/Kconfig
> > > > > > > > @@ -369,6 +369,15 @@ config K3_DMA
> > > > > > > > Support the DMA engine for Hisilicon K3 platform
> > > > > > > > devices.
> > > > > > > >
> > > > > > > > +config LOONGSON1_APB_DMA
> > > > > > > > + tristate "Loongson1 APB DMA support"
> > > > > > > > + depends on MACH_LOONGSON32 || COMPILE_TEST
> > > > > > > > + select DMA_ENGINE
> > > > > > > > + select DMA_VIRTUAL_CHANNELS
> > > > > > > > + help
> > > > > > > > + This selects support for the APB DMA controller in Loongson1 SoCs,
> > > > > > > > + which is required by Loongson1 NAND and audio support.
> > > > > > > Why not rename to LS1X_APB_DMA and put it just before LS2X_APB_DMA
> > > > > > > (and also the driver file name)?
> > > > > > >
> > > > > > So far all Kconfig entries of Loongson-1 drivers are named with the
> > > > > > keyword "LOONGSON1".
> > > > > > The same is true for these file names.
> > > > > > Therefore, I need to keep the consistency.
> > > > > But I see LS1X_IRQ in drivers/irqchip/Kconfig
> > > > >
> > > > Indeed, that's an exception, which was submitted by Jiaxun several years ago.
> > > > Actually, most drivers of Loongson family use the keyword "LOONGSON"
> > > > for Kconfig and "loongson" for filename.
> > > > Thus I take this keywork as the naming convention.
> > > But I think keeping consistency in a same subsystem is better than
> > > keeping consistency in a same SoC (but cross subsystems).
> > >
> > In my opinion, "LS*X" is too short and may be confused with other SoCs.
> > Meanwhile, there are only four drivers that use this keyword.
> > config I2C_LS2X
> > config LS2K_RESET
> > config LS2X_APB_DMA
> > config LS1X_IRQ
> > Then, my suggestion is to change these "LS*X" to "LOONGSON*" to get a
> > clear meaning.
> We have made a naming conversion some years before with Jiaxun.
> 1, Use "Loongson" for CPU in arch code;
> 2, Use "LS7A" or something like this for bridges and devices.
> 3, For drivers in SoC, if the driver is specific to Loongson-1, use
> LS1X, if it is to Loongson-2, use LS2X, if it is shared by both
> Loongson-1 and Loongson-2, use LOONGSON.
>
OK. But the doesn't the answer the question of confusion, such as
"Freescale LS1021A".
The same problem happens to the filenames.
./drivers/gpu/drm/nouveau/nvkm/nvfw/ls.c
./drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c
./drivers/gpu/drm/amd/amdgpu/lsdma_v6_0.c
./drivers/gpu/drm/amd/amdgpu/lsdma_v7_0.c
./arch/powerpc/platforms/embedded6xx/ls_uart.c
Regarding "LS*X" itself, it contains the wildcard character "X" which
itself is confusing.
Therefore, I don't think "LS*X" is clear enough.
On the other hand, I see "LOONGSON2_*" strings are still there.
In addition, some of "LOONGSON_" definitions are not applicable for
Loongson-1 at all, which breaks your convention.
config SND_SOC_LOONGSON_I2S_PCI /* Loongson-1 doesn't support I2S */
config SND_SOC_LOONGSON_CARD
config DWMAC_LOONGSON1
config DWMAC_LOONGSON /* This glue layer doesn't support Loongson-1 */
config COMMON_CLK_LOONGSON2
config RTC_DRV_LOONGSON
config SPI_LOONGSON_CORE
config SPI_LOONGSON_PCI /* N/A for Loongson-1 */
config SPI_LOONGSON_PLATFORM
config LOONGSON2_CPUFREQ
config DRM_LOONGSON /* N/A for Loongson-1 */
config LOONGSON1_WDT
config CLKSRC_LOONGSON1_PWM
config LOONGSON_LIOINTC /* N/A for Loongson-1 */
config LOONGSON_EIOINTC /* N/A for Loongson-1 */
config LOONGSON_HTPIC /* N/A for Loongson-1 */
config LOONGSON_HTVEC /* N/A for Loongson-1 */
config LOONGSON_PCH_PIC /* N/A for Loongson-1 */
config LOONGSON_PCH_MSI /* N/A for Loongson-1 */
config LOONGSON_PCH_LPC /* N/A for Loongson-1 */
config PINCTRL_LOONGSON2
config LOONGSON2_THERMAL
config LOONGSON2_GUTS
config LOONGSON2_PM
config LOONGSON_LAPTOP /* N/A for Loongson-1 */
config GPIO_LOONGSON
config GPIO_LOONGSON_64BIT -> N/A for Loongson-1
config GPIO_LOONGSON1
config PCI_LOONGSON
What's your plan about the above Kconfig entries?
Why can't we use LOONGSON1/LOONGSON2 for drivers?
> Huacai
>
> >
> > > Huacai
> > >
> > > >
> > > > > Huacai
> > > > >
> > > > > >
> > > > > >
> > > > > > > Huacai
> > > > > > >
> > > > > > > > +
> > > > > > > > config LPC18XX_DMAMUX
> > > > > > > > bool "NXP LPC18xx/43xx DMA MUX for PL080"
> > > > > > > > depends on ARCH_LPC18XX || COMPILE_TEST
> > > > > > > > diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> > > > > > > > index dfd40d14e408..b26f6677978a 100644
> > > > > > > > --- a/drivers/dma/Makefile
> > > > > > > > +++ b/drivers/dma/Makefile
> > > > > > > > @@ -47,6 +47,7 @@ obj-$(CONFIG_INTEL_IDMA64) += idma64.o
> > > > > > > > obj-$(CONFIG_INTEL_IOATDMA) += ioat/
> > > > > > > > obj-y += idxd/
> > > > > > > > obj-$(CONFIG_K3_DMA) += k3dma.o
> > > > > > > > +obj-$(CONFIG_LOONGSON1_APB_DMA) += loongson1-apb-dma.o
> > > > > > > > obj-$(CONFIG_LPC18XX_DMAMUX) += lpc18xx-dmamux.o
> > > > > > > > obj-$(CONFIG_LS2X_APB_DMA) += ls2x-apb-dma.o
> > > > > > > > obj-$(CONFIG_MILBEAUT_HDMAC) += milbeaut-hdmac.o
> > > > > > > > diff --git a/drivers/dma/loongson1-apb-dma.c b/drivers/dma/loongson1-apb-dma.c
> > > > > > > > new file mode 100644
> > > > > > > > index 000000000000..d474a2601e6e
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/drivers/dma/loongson1-apb-dma.c
> > > > > > > > @@ -0,0 +1,665 @@
> > > > > > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > > > > > +/*
> > > > > > > > + * Driver for Loongson-1 APB DMA Controller
> > > > > > > > + *
> > > > > > > > + * Copyright (C) 2015-2024 Keguang Zhang <keguang.zhang@gmail.com>
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +#include <linux/dmapool.h>
> > > > > > > > +#include <linux/dma-mapping.h>
> > > > > > > > +#include <linux/init.h>
> > > > > > > > +#include <linux/interrupt.h>
> > > > > > > > +#include <linux/iopoll.h>
> > > > > > > > +#include <linux/module.h>
> > > > > > > > +#include <linux/of.h>
> > > > > > > > +#include <linux/of_dma.h>
> > > > > > > > +#include <linux/platform_device.h>
> > > > > > > > +#include <linux/slab.h>
> > > > > > > > +
> > > > > > > > +#include "dmaengine.h"
> > > > > > > > +#include "virt-dma.h"
> > > > > > > > +
> > > > > > > > +/* Loongson-1 DMA Control Register */
> > > > > > > > +#define DMA_CTRL 0x0
> > > > > > > > +
> > > > > > > > +/* DMA Control Register Bits */
> > > > > > > > +#define DMA_STOP BIT(4)
> > > > > > > > +#define DMA_START BIT(3)
> > > > > > > > +#define DMA_ASK_VALID BIT(2)
> > > > > > > > +
> > > > > > > > +#define DMA_ADDR_MASK GENMASK(31, 6)
> > > > > > > > +
> > > > > > > > +/* DMA Next Field Bits */
> > > > > > > > +#define DMA_NEXT_VALID BIT(0)
> > > > > > > > +
> > > > > > > > +/* DMA Command Field Bits */
> > > > > > > > +#define DMA_RAM2DEV BIT(12)
> > > > > > > > +#define DMA_INT BIT(1)
> > > > > > > > +#define DMA_INT_MASK BIT(0)
> > > > > > > > +
> > > > > > > > +#define LS1X_DMA_MAX_CHANNELS 3
> > > > > > > > +
> > > > > > > > +/* Size of allocations for hardware descriptors */
> > > > > > > > +#define LS1X_DMA_DESCS_SIZE PAGE_SIZE
> > > > > > > > +#define LS1X_DMA_MAX_DESC \
> > > > > > > > + (LS1X_DMA_DESCS_SIZE / sizeof(struct ls1x_dma_hwdesc))
> > > > > > > > +
> > > > > > > > +struct ls1x_dma_hwdesc {
> > > > > > > > + u32 next; /* next descriptor address */
> > > > > > > > + u32 saddr; /* memory DMA address */
> > > > > > > > + u32 daddr; /* device DMA address */
> > > > > > > > + u32 length;
> > > > > > > > + u32 stride;
> > > > > > > > + u32 cycles;
> > > > > > > > + u32 cmd;
> > > > > > > > + u32 stats;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct ls1x_dma_desc {
> > > > > > > > + struct virt_dma_desc vdesc;
> > > > > > > > + enum dma_transfer_direction dir;
> > > > > > > > + enum dma_transaction_type type;
> > > > > > > > + unsigned int bus_width;
> > > > > > > > +
> > > > > > > > + unsigned int nr_descs; /* number of descriptors */
> > > > > > > > +
> > > > > > > > + struct ls1x_dma_hwdesc *hwdesc;
> > > > > > > > + dma_addr_t hwdesc_phys;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct ls1x_dma_chan {
> > > > > > > > + struct virt_dma_chan vchan;
> > > > > > > > + struct dma_pool *desc_pool;
> > > > > > > > + phys_addr_t src_addr;
> > > > > > > > + phys_addr_t dst_addr;
> > > > > > > > + enum dma_slave_buswidth src_addr_width;
> > > > > > > > + enum dma_slave_buswidth dst_addr_width;
> > > > > > > > +
> > > > > > > > + void __iomem *reg_base;
> > > > > > > > + int irq;
> > > > > > > > +
> > > > > > > > + struct ls1x_dma_desc *desc;
> > > > > > > > +
> > > > > > > > + struct ls1x_dma_hwdesc *curr_hwdesc;
> > > > > > > > + dma_addr_t curr_hwdesc_phys;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +struct ls1x_dma {
> > > > > > > > + struct dma_device ddev;
> > > > > > > > + void __iomem *reg_base;
> > > > > > > > +
> > > > > > > > + unsigned int nr_chans;
> > > > > > > > + struct ls1x_dma_chan chan[];
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +#define to_ls1x_dma_chan(dchan) \
> > > > > > > > + container_of(dchan, struct ls1x_dma_chan, vchan.chan)
> > > > > > > > +
> > > > > > > > +#define to_ls1x_dma_desc(vd) \
> > > > > > > > + container_of(vd, struct ls1x_dma_desc, vdesc)
> > > > > > > > +
> > > > > > > > +/* macros for registers read/write */
> > > > > > > > +#define chan_readl(chan, off) \
> > > > > > > > + readl((chan)->reg_base + (off))
> > > > > > > > +
> > > > > > > > +#define chan_writel(chan, off, val) \
> > > > > > > > + writel((val), (chan)->reg_base + (off))
> > > > > > > > +
> > > > > > > > +static inline struct device *chan2dev(struct dma_chan *chan)
> > > > > > > > +{
> > > > > > > > + return &chan->dev->device;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static inline int ls1x_dma_query(struct ls1x_dma_chan *chan,
> > > > > > > > + dma_addr_t *hwdesc_phys)
> > > > > > > > +{
> > > > > > > > + struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > > + int val, ret;
> > > > > > > > +
> > > > > > > > + val = *hwdesc_phys & DMA_ADDR_MASK;
> > > > > > > > + val |= DMA_ASK_VALID;
> > > > > > > > + val |= dchan->chan_id;
> > > > > > > > + chan_writel(chan, DMA_CTRL, val);
> > > > > > > > + ret = readl_poll_timeout_atomic(chan->reg_base + DMA_CTRL, val,
> > > > > > > > + !(val & DMA_ASK_VALID), 0, 3000);
> > > > > > > > + if (ret)
> > > > > > > > + dev_err(chan2dev(dchan), "failed to query DMA\n");
> > > > > > > > +
> > > > > > > > + return ret;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static inline int ls1x_dma_start(struct ls1x_dma_chan *chan,
> > > > > > > > + dma_addr_t *hwdesc_phys)
> > > > > > > > +{
> > > > > > > > + struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > > + int val, ret;
> > > > > > > > +
> > > > > > > > + dev_dbg(chan2dev(dchan), "cookie=%d, starting hwdesc=%x\n",
> > > > > > > > + dchan->cookie, *hwdesc_phys);
> > > > > > > > +
> > > > > > > > + val = *hwdesc_phys & DMA_ADDR_MASK;
> > > > > > > > + val |= DMA_START;
> > > > > > > > + val |= dchan->chan_id;
> > > > > > > > + chan_writel(chan, DMA_CTRL, val);
> > > > > > > > + ret = readl_poll_timeout(chan->reg_base + DMA_CTRL, val,
> > > > > > > > + !(val & DMA_START), 0, 3000);
> > > > > > > > + if (ret)
> > > > > > > > + dev_err(chan2dev(dchan), "failed to start DMA\n");
> > > > > > > > +
> > > > > > > > + return ret;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static inline void ls1x_dma_stop(struct ls1x_dma_chan *chan)
> > > > > > > > +{
> > > > > > > > + chan_writel(chan, DMA_CTRL, chan_readl(chan, DMA_CTRL) | DMA_STOP);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void ls1x_dma_free_chan_resources(struct dma_chan *dchan)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > +
> > > > > > > > + dma_free_coherent(chan2dev(dchan), sizeof(struct ls1x_dma_hwdesc),
> > > > > > > > + chan->curr_hwdesc, chan->curr_hwdesc_phys);
> > > > > > > > + vchan_free_chan_resources(&chan->vchan);
> > > > > > > > + dma_pool_destroy(chan->desc_pool);
> > > > > > > > + chan->desc_pool = NULL;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_alloc_chan_resources(struct dma_chan *dchan)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > +
> > > > > > > > + chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
> > > > > > > > + chan2dev(dchan),
> > > > > > > > + sizeof(struct ls1x_dma_hwdesc),
> > > > > > > > + __alignof__(struct ls1x_dma_hwdesc),
> > > > > > > > + 0);
> > > > > > > > + if (!chan->desc_pool)
> > > > > > > > + return -ENOMEM;
> > > > > > > > +
> > > > > > > > + /* allocate memory for querying current HW descriptor */
> > > > > > > > + dma_set_coherent_mask(chan2dev(dchan), DMA_BIT_MASK(32));
> > > > > > > > + chan->curr_hwdesc = dma_alloc_coherent(chan2dev(dchan),
> > > > > > > > + sizeof(struct ls1x_dma_hwdesc),
> > > > > > > > + &chan->curr_hwdesc_phys,
> > > > > > > > + GFP_KERNEL);
> > > > > > > > + if (!chan->curr_hwdesc)
> > > > > > > > + return -ENOMEM;
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void ls1x_dma_free_desc(struct virt_dma_desc *vdesc)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vdesc);
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(vdesc->tx.chan);
> > > > > > > > +
> > > > > > > > + dma_pool_free(chan->desc_pool, desc->hwdesc, desc->hwdesc_phys);
> > > > > > > > + chan->desc = NULL;
> > > > > > > > + kfree(desc);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static struct ls1x_dma_desc *
> > > > > > > > +ls1x_dma_alloc_desc(struct dma_chan *dchan, int sg_len,
> > > > > > > > + enum dma_transfer_direction direction,
> > > > > > > > + enum dma_transaction_type type)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + struct ls1x_dma_desc *desc;
> > > > > > > > +
> > > > > > > > + if (sg_len > LS1X_DMA_MAX_DESC) {
> > > > > > > > + dev_err(chan2dev(dchan), "sg_len %u exceeds limit %lu",
> > > > > > > > + sg_len, LS1X_DMA_MAX_DESC);
> > > > > > > > + return NULL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > > > > > > > + if (!desc)
> > > > > > > > + return NULL;
> > > > > > > > +
> > > > > > > > + /* allocate HW descriptors */
> > > > > > > > + desc->hwdesc = dma_pool_zalloc(chan->desc_pool, GFP_NOWAIT,
> > > > > > > > + &desc->hwdesc_phys);
> > > > > > > > + if (!desc->hwdesc) {
> > > > > > > > + dev_err(chan2dev(dchan), "failed to alloc HW descriptors\n");
> > > > > > > > + ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > > + return NULL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + desc->dir = direction;
> > > > > > > > + desc->type = type;
> > > > > > > > + desc->nr_descs = sg_len;
> > > > > > > > +
> > > > > > > > + return desc;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_setup_hwdescs(struct dma_chan *dchan,
> > > > > > > > + struct ls1x_dma_desc *desc,
> > > > > > > > + struct scatterlist *sgl, unsigned int sg_len)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + dma_addr_t next_hwdesc_phys = desc->hwdesc_phys;
> > > > > > > > +
> > > > > > > > + struct scatterlist *sg;
> > > > > > > > + unsigned int dev_addr, cmd, i;
> > > > > > > > +
> > > > > > > > + switch (desc->dir) {
> > > > > > > > + case DMA_MEM_TO_DEV:
> > > > > > > > + dev_addr = chan->dst_addr;
> > > > > > > > + desc->bus_width = chan->dst_addr_width;
> > > > > > > > + cmd = DMA_RAM2DEV | DMA_INT;
> > > > > > > > + break;
> > > > > > > > + case DMA_DEV_TO_MEM:
> > > > > > > > + dev_addr = chan->src_addr;
> > > > > > > > + desc->bus_width = chan->src_addr_width;
> > > > > > > > + cmd = DMA_INT;
> > > > > > > > + break;
> > > > > > > > + default:
> > > > > > > > + dev_err(chan2dev(dchan), "unsupported DMA direction: %s\n",
> > > > > > > > + dmaengine_get_direction_text(desc->dir));
> > > > > > > > + return -EINVAL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + /* setup HW descriptors */
> > > > > > > > + for_each_sg(sgl, sg, sg_len, i) {
> > > > > > > > + dma_addr_t buf_addr = sg_dma_address(sg);
> > > > > > > > + size_t buf_len = sg_dma_len(sg);
> > > > > > > > + struct ls1x_dma_hwdesc *hwdesc = &desc->hwdesc[i];
> > > > > > > > +
> > > > > > > > + if (!is_dma_copy_aligned(dchan->device, buf_addr, 0, buf_len)) {
> > > > > > > > + dev_err(chan2dev(dchan), "buffer is not aligned!\n");
> > > > > > > > + return -EINVAL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + hwdesc->saddr = buf_addr;
> > > > > > > > + hwdesc->daddr = dev_addr;
> > > > > > > > + hwdesc->length = buf_len / desc->bus_width;
> > > > > > > > + hwdesc->stride = 0;
> > > > > > > > + hwdesc->cycles = 1;
> > > > > > > > + hwdesc->cmd = cmd;
> > > > > > > > +
> > > > > > > > + if (i) {
> > > > > > > > + next_hwdesc_phys += sizeof(*hwdesc);
> > > > > > > > + desc->hwdesc[i - 1].next = next_hwdesc_phys
> > > > > > > > + | DMA_NEXT_VALID;
> > > > > > > > + }
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + if (desc->type == DMA_CYCLIC)
> > > > > > > > + desc->hwdesc[i - 1].next = desc->hwdesc_phys | DMA_NEXT_VALID;
> > > > > > > > +
> > > > > > > > + for_each_sg(sgl, sg, sg_len, i) {
> > > > > > > > + struct ls1x_dma_hwdesc *hwdesc = &desc->hwdesc[i];
> > > > > > > > +
> > > > > > > > + print_hex_dump_debug("HW DESC: ", DUMP_PREFIX_OFFSET, 16, 4,
> > > > > > > > + hwdesc, sizeof(*hwdesc), false);
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static struct dma_async_tx_descriptor *
> > > > > > > > +ls1x_dma_prep_slave_sg(struct dma_chan *dchan,
> > > > > > > > + struct scatterlist *sgl, unsigned int sg_len,
> > > > > > > > + enum dma_transfer_direction direction,
> > > > > > > > + unsigned long flags, void *context)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + struct ls1x_dma_desc *desc;
> > > > > > > > +
> > > > > > > > + dev_dbg(chan2dev(dchan), "sg_len=%u flags=0x%lx dir=%s\n",
> > > > > > > > + sg_len, flags, dmaengine_get_direction_text(direction));
> > > > > > > > +
> > > > > > > > + desc = ls1x_dma_alloc_desc(dchan, sg_len, direction, DMA_SLAVE);
> > > > > > > > + if (!desc)
> > > > > > > > + return NULL;
> > > > > > > > +
> > > > > > > > + if (ls1x_dma_setup_hwdescs(dchan, desc, sgl, sg_len)) {
> > > > > > > > + ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > > + return NULL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static struct dma_async_tx_descriptor *
> > > > > > > > +ls1x_dma_prep_dma_cyclic(struct dma_chan *dchan,
> > > > > > > > + dma_addr_t buf_addr, size_t buf_len, size_t period_len,
> > > > > > > > + enum dma_transfer_direction direction,
> > > > > > > > + unsigned long flags)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + struct ls1x_dma_desc *desc;
> > > > > > > > + struct scatterlist *sgl;
> > > > > > > > + unsigned int sg_len;
> > > > > > > > + unsigned int i;
> > > > > > > > +
> > > > > > > > + dev_dbg(chan2dev(dchan),
> > > > > > > > + "buf_len=%d period_len=%zu flags=0x%lx dir=%s\n", buf_len,
> > > > > > > > + period_len, flags, dmaengine_get_direction_text(direction));
> > > > > > > > +
> > > > > > > > + sg_len = buf_len / period_len;
> > > > > > > > + desc = ls1x_dma_alloc_desc(dchan, sg_len, direction, DMA_CYCLIC);
> > > > > > > > + if (!desc)
> > > > > > > > + return NULL;
> > > > > > > > +
> > > > > > > > + /* allocate the scatterlist */
> > > > > > > > + sgl = kmalloc_array(sg_len, sizeof(*sgl), GFP_NOWAIT);
> > > > > > > > + if (!sgl)
> > > > > > > > + return NULL;
> > > > > > > > +
> > > > > > > > + sg_init_table(sgl, sg_len);
> > > > > > > > + for (i = 0; i < sg_len; ++i) {
> > > > > > > > + sg_set_page(&sgl[i], pfn_to_page(PFN_DOWN(buf_addr)),
> > > > > > > > + period_len, offset_in_page(buf_addr));
> > > > > > > > + sg_dma_address(&sgl[i]) = buf_addr;
> > > > > > > > + sg_dma_len(&sgl[i]) = period_len;
> > > > > > > > + buf_addr += period_len;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + if (ls1x_dma_setup_hwdescs(dchan, desc, sgl, sg_len)) {
> > > > > > > > + ls1x_dma_free_desc(&desc->vdesc);
> > > > > > > > + return NULL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + kfree(sgl);
> > > > > > > > +
> > > > > > > > + return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_slave_config(struct dma_chan *dchan,
> > > > > > > > + struct dma_slave_config *config)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > +
> > > > > > > > + chan->src_addr = config->src_addr;
> > > > > > > > + chan->src_addr_width = config->src_addr_width;
> > > > > > > > + chan->dst_addr = config->dst_addr;
> > > > > > > > + chan->dst_addr_width = config->dst_addr_width;
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_pause(struct dma_chan *dchan)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + unsigned long flags;
> > > > > > > > + int ret;
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > + ret = ls1x_dma_query(chan, &chan->curr_hwdesc_phys);
> > > > > > > > + if (!ret)
> > > > > > > > + ls1x_dma_stop(chan);
> > > > > > > > + spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > +
> > > > > > > > + return ret;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_resume(struct dma_chan *dchan)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + unsigned long flags;
> > > > > > > > + int ret;
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > + ret = ls1x_dma_start(chan, &chan->curr_hwdesc_phys);
> > > > > > > > + spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > +
> > > > > > > > + return ret;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_terminate_all(struct dma_chan *dchan)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + unsigned long flags;
> > > > > > > > + LIST_HEAD(head);
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > + ls1x_dma_stop(chan);
> > > > > > > > + vchan_get_all_descriptors(&chan->vchan, &head);
> > > > > > > > + spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > +
> > > > > > > > + vchan_dma_desc_free_list(&chan->vchan, &head);
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> > > > > > > > + dma_cookie_t cookie,
> > > > > > > > + struct dma_tx_state *state)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + struct virt_dma_desc *vdesc;
> > > > > > > > + enum dma_status status;
> > > > > > > > + size_t bytes = 0;
> > > > > > > > + unsigned long flags;
> > > > > > > > +
> > > > > > > > + status = dma_cookie_status(dchan, cookie, state);
> > > > > > > > + if (status == DMA_COMPLETE)
> > > > > > > > + return status;
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > + vdesc = vchan_find_desc(&chan->vchan, cookie);
> > > > > > > > + if (chan->desc && cookie == chan->desc->vdesc.tx.cookie) {
> > > > > > > > + struct ls1x_dma_desc *desc = chan->desc;
> > > > > > > > + int i;
> > > > > > > > +
> > > > > > > > + if (ls1x_dma_query(chan, &chan->curr_hwdesc_phys))
> > > > > > > > + return status;
> > > > > > > > +
> > > > > > > > + /* locate the current HW descriptor */
> > > > > > > > + for (i = 0; i < desc->nr_descs; i++)
> > > > > > > > + if (desc->hwdesc[i].next == chan->curr_hwdesc->next)
> > > > > > > > + break;
> > > > > > > > +
> > > > > > > > + /* count the residues */
> > > > > > > > + for (; i < desc->nr_descs; i++)
> > > > > > > > + bytes += desc->hwdesc[i].length * desc->bus_width;
> > > > > > > > +
> > > > > > > > + dma_set_residue(state, bytes);
> > > > > > > > + }
> > > > > > > > + spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > +
> > > > > > > > + return status;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void ls1x_dma_issue_pending(struct dma_chan *dchan)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = to_ls1x_dma_chan(dchan);
> > > > > > > > + struct virt_dma_desc *vdesc;
> > > > > > > > + unsigned long flags;
> > > > > > > > +
> > > > > > > > + spin_lock_irqsave(&chan->vchan.lock, flags);
> > > > > > > > + if (vchan_issue_pending(&chan->vchan) && !chan->desc) {
> > > > > > > > + vdesc = vchan_next_desc(&chan->vchan);
> > > > > > > > + if (!vdesc) {
> > > > > > > > + chan->desc = NULL;
> > > > > > > > + return;
> > > > > > > > + }
> > > > > > > > + chan->desc = to_ls1x_dma_desc(vdesc);
> > > > > > > > + ls1x_dma_start(chan, &chan->desc->hwdesc_phys);
> > > > > > > > + }
> > > > > > > > + spin_unlock_irqrestore(&chan->vchan.lock, flags);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static irqreturn_t ls1x_dma_irq_handler(int irq, void *data)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma_chan *chan = data;
> > > > > > > > + struct ls1x_dma_desc *desc = chan->desc;
> > > > > > > > + struct dma_chan *dchan = &chan->vchan.chan;
> > > > > > > > +
> > > > > > > > + if (!desc) {
> > > > > > > > + dev_warn(chan2dev(dchan),
> > > > > > > > + "IRQ %d with no active descriptor on channel %d\n",
> > > > > > > > + irq, dchan->chan_id);
> > > > > > > > + return IRQ_NONE;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + dev_dbg(chan2dev(dchan), "DMA IRQ %d on channel %d\n", irq,
> > > > > > > > + dchan->chan_id);
> > > > > > > > +
> > > > > > > > + spin_lock(&chan->vchan.lock);
> > > > > > > > +
> > > > > > > > + if (desc->type == DMA_CYCLIC) {
> > > > > > > > + vchan_cyclic_callback(&desc->vdesc);
> > > > > > > > + } else {
> > > > > > > > + list_del(&desc->vdesc.node);
> > > > > > > > + vchan_cookie_complete(&desc->vdesc);
> > > > > > > > + chan->desc = NULL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + spin_unlock(&chan->vchan.lock);
> > > > > > > > + return IRQ_HANDLED;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_chan_probe(struct platform_device *pdev,
> > > > > > > > + struct ls1x_dma *dma, int chan_id)
> > > > > > > > +{
> > > > > > > > + struct device *dev = &pdev->dev;
> > > > > > > > + struct ls1x_dma_chan *chan = &dma->chan[chan_id];
> > > > > > > > + char pdev_irqname[4];
> > > > > > > > + char *irqname;
> > > > > > > > + int ret;
> > > > > > > > +
> > > > > > > > + sprintf(pdev_irqname, "ch%u", chan_id);
> > > > > > > > + chan->irq = platform_get_irq_byname(pdev, pdev_irqname);
> > > > > > > > + if (chan->irq < 0)
> > > > > > > > + return -ENODEV;
> > > > > > > > +
> > > > > > > > + irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:%s",
> > > > > > > > + dev_name(dev), pdev_irqname);
> > > > > > > > + if (!irqname)
> > > > > > > > + return -ENOMEM;
> > > > > > > > +
> > > > > > > > + ret = devm_request_irq(dev, chan->irq, ls1x_dma_irq_handler,
> > > > > > > > + IRQF_SHARED, irqname, chan);
> > > > > > > > + if (ret)
> > > > > > > > + return dev_err_probe(dev, ret,
> > > > > > > > + "failed to request IRQ %u!\n", chan->irq);
> > > > > > > > +
> > > > > > > > + chan->reg_base = dma->reg_base;
> > > > > > > > + chan->vchan.desc_free = ls1x_dma_free_desc;
> > > > > > > > + vchan_init(&chan->vchan, &dma->ddev);
> > > > > > > > + dev_info(dev, "%s (irq %d) initialized\n", pdev_irqname, chan->irq);
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void ls1x_dma_chan_remove(struct ls1x_dma *dma, int chan_id)
> > > > > > > > +{
> > > > > > > > + struct device *dev = dma->ddev.dev;
> > > > > > > > + struct ls1x_dma_chan *chan = &dma->chan[chan_id];
> > > > > > > > +
> > > > > > > > + devm_free_irq(dev, chan->irq, chan);
> > > > > > > > + list_del(&chan->vchan.chan.device_node);
> > > > > > > > + tasklet_kill(&chan->vchan.task);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static int ls1x_dma_probe(struct platform_device *pdev)
> > > > > > > > +{
> > > > > > > > + struct device *dev = &pdev->dev;
> > > > > > > > + struct dma_device *ddev;
> > > > > > > > + struct ls1x_dma *dma;
> > > > > > > > + int nr_chans, ret, i;
> > > > > > > > +
> > > > > > > > + nr_chans = platform_irq_count(pdev);
> > > > > > > > + if (nr_chans <= 0)
> > > > > > > > + return nr_chans;
> > > > > > > > + if (nr_chans > LS1X_DMA_MAX_CHANNELS)
> > > > > > > > + return dev_err_probe(dev, -EINVAL,
> > > > > > > > + "nr_chans=%d exceeds the maximum\n",
> > > > > > > > + nr_chans);
> > > > > > > > +
> > > > > > > > + dma = devm_kzalloc(dev, struct_size(dma, chan, nr_chans), GFP_KERNEL);
> > > > > > > > + if (!dma)
> > > > > > > > + return -ENOMEM;
> > > > > > > > +
> > > > > > > > + /* initialize DMA device */
> > > > > > > > + dma->reg_base = devm_platform_ioremap_resource(pdev, 0);
> > > > > > > > + if (IS_ERR(dma->reg_base))
> > > > > > > > + return PTR_ERR(dma->reg_base);
> > > > > > > > +
> > > > > > > > + ddev = &dma->ddev;
> > > > > > > > + ddev->dev = dev;
> > > > > > > > + ddev->copy_align = DMAENGINE_ALIGN_4_BYTES;
> > > > > > > > + ddev->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > > > > > + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > > > > > > + ddev->dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> > > > > > > > + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> > > > > > > > + ddev->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> > > > > > > > + ddev->max_sg_burst = LS1X_DMA_MAX_DESC;
> > > > > > > > + ddev->residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> > > > > > > > + ddev->device_alloc_chan_resources = ls1x_dma_alloc_chan_resources;
> > > > > > > > + ddev->device_free_chan_resources = ls1x_dma_free_chan_resources;
> > > > > > > > + ddev->device_prep_slave_sg = ls1x_dma_prep_slave_sg;
> > > > > > > > + ddev->device_prep_dma_cyclic = ls1x_dma_prep_dma_cyclic;
> > > > > > > > + ddev->device_config = ls1x_dma_slave_config;
> > > > > > > > + ddev->device_pause = ls1x_dma_pause;
> > > > > > > > + ddev->device_resume = ls1x_dma_resume;
> > > > > > > > + ddev->device_terminate_all = ls1x_dma_terminate_all;
> > > > > > > > + ddev->device_tx_status = ls1x_dma_tx_status;
> > > > > > > > + ddev->device_issue_pending = ls1x_dma_issue_pending;
> > > > > > > > +
> > > > > > > > + dma_cap_set(DMA_SLAVE, ddev->cap_mask);
> > > > > > > > + INIT_LIST_HEAD(&ddev->channels);
> > > > > > > > +
> > > > > > > > + /* initialize DMA channels */
> > > > > > > > + for (i = 0; i < nr_chans; i++) {
> > > > > > > > + ret = ls1x_dma_chan_probe(pdev, dma, i);
> > > > > > > > + if (ret)
> > > > > > > > + return ret;
> > > > > > > > + }
> > > > > > > > + dma->nr_chans = nr_chans;
> > > > > > > > +
> > > > > > > > + ret = dmaenginem_async_device_register(ddev);
> > > > > > > > + if (ret) {
> > > > > > > > + dev_err(dev, "failed to register DMA device! %d\n", ret);
> > > > > > > > + return ret;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + ret =
> > > > > > > > + of_dma_controller_register(dev->of_node, of_dma_xlate_by_chan_id,
> > > > > > > > + ddev);
> > > > > > > > + if (ret) {
> > > > > > > > + dev_err(dev, "failed to register DMA controller! %d\n", ret);
> > > > > > > > + return ret;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + platform_set_drvdata(pdev, dma);
> > > > > > > > + dev_info(dev, "Loongson1 DMA driver registered\n");
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void ls1x_dma_remove(struct platform_device *pdev)
> > > > > > > > +{
> > > > > > > > + struct ls1x_dma *dma = platform_get_drvdata(pdev);
> > > > > > > > + int i;
> > > > > > > > +
> > > > > > > > + of_dma_controller_free(pdev->dev.of_node);
> > > > > > > > +
> > > > > > > > + for (i = 0; i < dma->nr_chans; i++)
> > > > > > > > + ls1x_dma_chan_remove(dma, i);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static const struct of_device_id ls1x_dma_match[] = {
> > > > > > > > + { .compatible = "loongson,ls1b-apbdma" },
> > > > > > > > + { .compatible = "loongson,ls1c-apbdma" },
> > > > > > > > + { /* sentinel */ }
> > > > > > > > +};
> > > > > > > > +MODULE_DEVICE_TABLE(of, ls1x_dma_match);
> > > > > > > > +
> > > > > > > > +static struct platform_driver ls1x_dma_driver = {
> > > > > > > > + .probe = ls1x_dma_probe,
> > > > > > > > + .remove_new = ls1x_dma_remove,
> > > > > > > > + .driver = {
> > > > > > > > + .name = KBUILD_MODNAME,
> > > > > > > > + .of_match_table = ls1x_dma_match,
> > > > > > > > + },
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > +module_platform_driver(ls1x_dma_driver);
> > > > > > > > +
> > > > > > > > +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
> > > > > > > > +MODULE_DESCRIPTION("Loongson-1 APB DMA Controller driver");
> > > > > > > > +MODULE_LICENSE("GPL");
> > > > > > > >
> > > > > > > > --
> > > > > > > > 2.40.1
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best regards,
> > > > > >
> > > > > > Keguang Zhang
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > >
> > > > Keguang Zhang
> >
> >
> >
> > --
> > Best regards,
> >
> > Keguang Zhang
--
Best regards,
Keguang Zhang
^ permalink raw reply
* Re: [PATCH v5 2/2] backlight: Add new lm3509 backlight driver
From: Daniel Thompson @ 2024-04-02 10:52 UTC (permalink / raw)
To: Patrick Gansterer
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev,
Lee Jones, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Sam Ravnborg
In-Reply-To: <20240330145931.729116-3-paroga@paroga.com>
On Sat, Mar 30, 2024 at 03:59:25PM +0100, Patrick Gansterer wrote:
> This is a general driver for LM3509 backlight chip of TI.
> LM3509 is High Efficiency Boost for White LEDs and/or OLED Displays with
> Dual Current Sinks. This driver supports OLED/White LED select, brightness
> control and sub/main control.
> The datasheet can be found at http://www.ti.com/product/lm3509.
>
> Signed-off-by: Patrick Gansterer <paroga@paroga.com>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ permalink raw reply
* Re: [PATCH 1/1] arm64: dts: imx8-ss-conn: fix usdhc wrong lpcg clock order
From: Shawn Guo @ 2024-04-02 10:56 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Dong Aisheng,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <20240322164706.2626088-1-Frank.Li@nxp.com>
On Fri, Mar 22, 2024 at 12:47:05PM -0400, Frank Li wrote:
> The actual clock show wrong frequency:
>
> echo on >/sys/devices/platform/bus\@5b000000/5b010000.mmc/power/control
> cat /sys/kernel/debug/mmc0/ios
>
> clock: 200000000 Hz
> actual clock: 166000000 Hz
> ^^^^^^^^^
> .....
>
> According to
>
> sdhc0_lpcg: clock-controller@5b200000 {
> compatible = "fsl,imx8qxp-lpcg";
> reg = <0x5b200000 0x10000>;
> #clock-cells = <1>;
> clocks = <&clk IMX_SC_R_SDHC_0 IMX_SC_PM_CLK_PER>,
> <&conn_ipg_clk>, <&conn_axi_clk>;
> clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>,
> <IMX_LPCG_CLK_5>;
> clock-output-names = "sdhc0_lpcg_per_clk",
> "sdhc0_lpcg_ipg_clk",
> "sdhc0_lpcg_ahb_clk";
> power-domains = <&pd IMX_SC_R_SDHC_0>;
> }
>
> "per_clk" should be IMX_LPCG_CLK_0 instead of IMX_LPCG_CLK_5.
>
> After correct clocks order:
>
> echo on >/sys/devices/platform/bus\@5b000000/5b010000.mmc/power/control
> cat /sys/kernel/debug/mmc0/ios
>
> clock: 200000000 Hz
> actual clock: 198000000 Hz
> ^^^^^^^^
> ...
>
> Fixes: 16c4ea7501b1 ("arm64: dts: imx8: switch to new lpcg clock binding")
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Applied, thanks!
^ permalink raw reply
* [PATCH] dt-bindings: mfd: syscon: Add ti,am62p-cpsw-mac-efuse compatible
From: Siddharth Vadapalli @ 2024-04-02 10:57 UTC (permalink / raw)
To: lee, robh, krzk+dt, conor+dt
Cc: devicetree, linux-kernel, linux-arm-kernel, srk, s-vadapalli
The CTRLMMR_MAC_IDx registers within the CTRL_MMR space of TI's AM62p SoC
contain the MAC Address programmed in the eFuse. Add compatible for
allowing the CPSW driver to obtain a regmap for the CTRLMMR_MAC_IDx
registers within the System Controller device-tree node. The default MAC
Address for the interface corresponding to the first MAC port will be set
to the value programmed in the eFuse.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
This patch is based on linux-next tagged next-20240402.
Regards,
Siddharth.
Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 9d55bee155ce..4936ac0b5936 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -73,6 +73,7 @@ properties:
- rockchip,rv1126-qos
- starfive,jh7100-sysmain
- ti,am62-usb-phy-ctrl
+ - ti,am62p-cpsw-mac-efuse
- ti,am654-dss-oldi-io-ctrl
- ti,am654-serdes-ctrl
- ti,j784s4-pcie-ctrl
--
2.40.1
^ permalink raw reply related
* RE: [EXT] Re: [PATCH v10 08/11] arm64: dts: imx93: add usb nodes
From: Xu Yang @ 2024-04-02 10:57 UTC (permalink / raw)
To: Shawn Guo
Cc: gregkh@linuxfoundation.org, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
conor+dt@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
peter.chen@kernel.org, Jun Li, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
imx@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <ZgvEXZTOHUv+GGeH@dragon>
Hi Shawn,
>
> On Thu, Mar 21, 2024 at 04:14:36PM +0800, Xu Yang wrote:
> > There are 2 USB controllers on i.MX93. Add them.
> >
> > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa9352LA/CA
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> > - fix format as suggested by Alexander
> > - change compatible from fsl,imx8mm-usb to fsl,imx93-usb
> > Changes in v3:
> > - replace deprecated fsl,usbphy with phys as suggested by Alexander
> > - reorder nodes
> > Changes in v4:
> > - fix the alignment
> > Changes in v5:
> > - rename usb_wakeup_clk to usb_wakeup
> > Changes in v6:
> > - rename usb_ctrl_root_clk to usb_ctrl_root
> > Changes in v7:
> > - no changes
> > Changes in v8:
> > - no changes
> > Changes in v9:
> > - no changes
> > Changes in v10:
> > - no changes
> > ---
> > arch/arm64/boot/dts/freescale/imx93.dtsi | 58 ++++++++++++++++++++++++
> > 1 file changed, 58 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > index 8f2e7c42ad6e..4a7efccb4f67 100644
> > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > @@ -183,6 +183,20 @@ mqs2: mqs2 {
> > status = "disabled";
> > };
> >
> > + usbphynop1: usbphynop1 {
> > + compatible = "usb-nop-xceiv";
> > + #phy-cells = <0>;
> > + clocks = <&clk IMX93_CLK_USB_PHY_BURUNIN>;
> > + clock-names = "main_clk";
> > + };
> > +
> > + usbphynop2: usbphynop2 {
> > + compatible = "usb-nop-xceiv";
> > + #phy-cells = <0>;
> > + clocks = <&clk IMX93_CLK_USB_PHY_BURUNIN>;
> > + clock-names = "main_clk";
> > + };
> > +
> > soc@0 {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > @@ -1167,6 +1181,50 @@ media_blk_ctrl: system-controller@4ac10000 {
> > status = "disabled";
> > };
> >
> > + usbotg1: usb@4c100000 {
> > + compatible = "fsl,imx93-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
> > + reg = <0x4c100000 0x200>;
> > + interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX93_CLK_USB_CONTROLLER_GATE>,
> > + <&clk IMX93_CLK_HSIO_32K_GATE>;
> > + clock-names = "usb_ctrl_root", "usb_wakeup";
> > + assigned-clocks = <&clk IMX93_CLK_HSIO>;
> > + assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
> > + assigned-clock-rates = <133000000>;
> > + phys = <&usbphynop1>;
> > + fsl,usbmisc = <&usbmisc1 0>;
> > + status = "disabled";
> > + };
> > +
> > + usbmisc1: usbmisc@4c100200 {
> > + compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc",
> > + "fsl,imx6q-usbmisc";
> > + reg = <0x4c100200 0x200>;
> > + #index-cells = <1>;
>
> Do we still need this '#index-cells' property? I see it's being marked
> as deprecated in bindings doc.
Sorry, the driver still needs fetch the value of this property so far. Otherwise,
the driver will probe failed. We still need some time to totally retire this property.
Thanks,
Xu Yang
>
> Shawn
>
> > + };
> > +
> > + usbotg2: usb@4c200000 {
> > + compatible = "fsl,imx93-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
> > + reg = <0x4c200000 0x200>;
> > + interrupts = <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clk IMX93_CLK_USB_CONTROLLER_GATE>,
> > + <&clk IMX93_CLK_HSIO_32K_GATE>;
> > + clock-names = "usb_ctrl_root", "usb_wakeup";
> > + assigned-clocks = <&clk IMX93_CLK_HSIO>;
> > + assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>;
> > + assigned-clock-rates = <133000000>;
> > + phys = <&usbphynop2>;
> > + fsl,usbmisc = <&usbmisc2 0>;
> > + status = "disabled";
> > + };
> > +
> > + usbmisc2: usbmisc@4c200200 {
> > + compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc",
> > + "fsl,imx6q-usbmisc";
> > + reg = <0x4c200200 0x200>;
> > + #index-cells = <1>;
> > + };
> > +
> > ddr-pmu@4e300dc0 {
> > compatible = "fsl,imx93-ddr-pmu";
> > reg = <0x4e300dc0 0x200>;
> > --
> > 2.34.1
> >
^ permalink raw reply
* [PATCH 1/2] drm/bridge: lt8912b: add support for P/N pin swap
From: Alexandru Ardelean @ 2024-04-02 10:59 UTC (permalink / raw)
To: linux-kernel, dri-devel, devicetree
Cc: adrien.grassein, andrzej.hajda, neil.armstrong, rfoss,
Laurent.pinchart, jonas, jernej.skrabec, airlied, daniel,
maarten.lankhorst, mripard, tzimmermann, robh,
krzysztof.kozlowski+dt, conor+dt, stefan.eichenberger,
francesco.dolcini, marius.muresan, irina.muresan,
Alexandru Ardelean
On some HW designs, it's easier for the layout if the P/N pins are swapped.
In those cases, we need to adjust (for this) by configuring the MIPI analog
registers differently. Specifically, register 0x3e needs to be 0xf6
(instead of 0xd6).
This change adds a 'lontium,pn-swap' device-tree property to configure the
MIPI analog registers for P/N swap.
Signed-off-by: Alexandru Ardelean <alex@shruggie.ro>
---
drivers/gpu/drm/bridge/lontium-lt8912b.c | 25 +++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c
index 4b2ae27f0a57f..154126bb922b4 100644
--- a/drivers/gpu/drm/bridge/lontium-lt8912b.c
+++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c
@@ -47,6 +47,7 @@ struct lt8912 {
u8 data_lanes;
bool is_power_on;
+ bool do_pn_swap;
};
static int lt8912_write_init_config(struct lt8912 *lt)
@@ -78,15 +79,31 @@ static int lt8912_write_init_config(struct lt8912 *lt)
{0x55, 0x44},
{0x57, 0x01},
{0x5a, 0x02},
-
- /*MIPI Analog*/
+ };
+ const struct reg_sequence mipi_analog_seq[] = {
{0x3e, 0xd6},
{0x3f, 0xd4},
{0x41, 0x3c},
{0xB2, 0x00},
};
+ const struct reg_sequence mipi_analog_pn_swap_seq[] = {
+ {0x3e, 0xf6},
+ {0x3f, 0xd4},
+ {0x41, 0x3c},
+ {0xB2, 0x00},
+ };
+ int ret;
- return regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq));
+ ret = regmap_multi_reg_write(lt->regmap[I2C_MAIN], seq, ARRAY_SIZE(seq));
+ if (ret < 0)
+ return ret;
+
+ if (!lt->do_pn_swap)
+ return regmap_multi_reg_write(lt->regmap[I2C_MAIN], mipi_analog_seq,
+ ARRAY_SIZE(mipi_analog_seq));
+
+ return regmap_multi_reg_write(lt->regmap[I2C_MAIN], mipi_analog_pn_swap_seq,
+ ARRAY_SIZE(mipi_analog_pn_swap_seq));
}
static int lt8912_write_mipi_basic_config(struct lt8912 *lt)
@@ -702,6 +719,8 @@ static int lt8912_parse_dt(struct lt8912 *lt)
}
lt->gp_reset = gp_reset;
+ lt->do_pn_swap = device_property_read_bool(dev, "lontium,pn-swap");
+
data_lanes = drm_of_get_data_lanes_count_ep(dev->of_node, 0, -1, 1, 4);
if (data_lanes < 0) {
dev_err(lt->dev, "%s: Bad data-lanes property\n", __func__);
--
2.44.0
^ permalink raw reply related
* [PATCH 2/2] dt-bindings: display: bridge: lt8912b: document 'lontium,pn-swap' property
From: Alexandru Ardelean @ 2024-04-02 10:59 UTC (permalink / raw)
To: linux-kernel, dri-devel, devicetree
Cc: adrien.grassein, andrzej.hajda, neil.armstrong, rfoss,
Laurent.pinchart, jonas, jernej.skrabec, airlied, daniel,
maarten.lankhorst, mripard, tzimmermann, robh,
krzysztof.kozlowski+dt, conor+dt, stefan.eichenberger,
francesco.dolcini, marius.muresan, irina.muresan,
Alexandru Ardelean
In-Reply-To: <20240402105925.905144-1-alex@shruggie.ro>
On some HW designs, it's easier for the layout if the P/N pins are swapped.
The driver currently has a DT property to do that.
This change documents the 'lontium,pn-swap' property.
Signed-off-by: Alexandru Ardelean <alex@shruggie.ro>
---
.../devicetree/bindings/display/bridge/lontium,lt8912b.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt8912b.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt8912b.yaml
index 2cef252157985..3a804926b288a 100644
--- a/Documentation/devicetree/bindings/display/bridge/lontium,lt8912b.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt8912b.yaml
@@ -24,6 +24,12 @@ properties:
maxItems: 1
description: GPIO connected to active high RESET pin.
+ lontium,pn-swap:
+ description: Swap the polarities of the P/N pins in software.
+ On some HW designs, the layout is simplified if the P/N pins
+ are inverted.
+ type: boolean
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
From: Varadarajan Narayanan @ 2024-04-02 11:02 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, quic_anusha,
linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <CAA8EJpo=TMhu+Te+JE0cQzmjLOTDPi-Vv-h5Bch0Wfr_7iVi2w@mail.gmail.com>
On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > able to release the resources properly.
> > >
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > > v5: Introduced devm_icc_clk_register
> > > ---
> > > drivers/interconnect/icc-clk.c | 29 +++++++++++++++++++++++++++++
> > > include/linux/interconnect-clk.h | 4 ++++
> > > 2 files changed, 33 insertions(+)
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Wait. Actually,
>
> Unreviewed-by: me
>
> Please return int from devm_icc_clk_register instead of returning the pointer.
Wouldn't returning int break the general assumption that
devm_foo(), returns the same type as foo(). For example
devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?
Thanks
Varada
^ permalink raw reply
* Re: [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock controller driver
From: Dmitry Rokosov @ 2024-04-02 11:05 UTC (permalink / raw)
To: Jerome Brunet
Cc: neil.armstrong, mturquette, sboyd, robh+dt,
krzysztof.kozlowski+dt, khilman, martin.blumenstingl, kernel,
rockosov, linux-amlogic, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <1jv850hyvm.fsf@starbuckisacylon.baylibre.com>
Hello Jerome,
On Tue, Apr 02, 2024 at 11:35:49AM +0200, Jerome Brunet wrote:
>
> On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
>
> > The CPU clock controller plays a general role in the Amlogic A1 SoC
> > family by generating CPU clocks. As an APB slave module, it offers the
> > capability to inherit the CPU clock from two sources: the internal fixed
> > clock known as 'cpu fixed clock' and the external input provided by the
> > A1 PLL clock controller, referred to as 'syspll'.
> >
> > It is important for the driver to handle cpu_clk rate switching
> > effectively by transitioning to the CPU fixed clock to avoid any
> > potential execution freezes.
> >
> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> > ---
> > drivers/clk/meson/Kconfig | 10 ++
> > drivers/clk/meson/Makefile | 1 +
> > drivers/clk/meson/a1-cpu.c | 324 +++++++++++++++++++++++++++++++++++++
> > drivers/clk/meson/a1-cpu.h | 16 ++
> > 4 files changed, 351 insertions(+)
> > create mode 100644 drivers/clk/meson/a1-cpu.c
> > create mode 100644 drivers/clk/meson/a1-cpu.h
> >
> > diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> > index 80c4a18c83d2..148d4495eee3 100644
> > --- a/drivers/clk/meson/Kconfig
> > +++ b/drivers/clk/meson/Kconfig
> > @@ -111,6 +111,16 @@ config COMMON_CLK_AXG_AUDIO
> > Support for the audio clock controller on AmLogic A113D devices,
> > aka axg, Say Y if you want audio subsystem to work.
> >
> > +config COMMON_CLK_A1_CPU
> > + tristate "Amlogic A1 SoC CPU controller support"
> > + depends on ARM64
> > + select COMMON_CLK_MESON_REGMAP
> > + select COMMON_CLK_MESON_CLKC_UTILS
> > + help
> > + Support for the CPU clock controller on Amlogic A113L based
> > + device, A1 SoC Family. Say Y if you want A1 CPU clock controller
> > + to work.
> > +
> > config COMMON_CLK_A1_PLL
> > tristate "Amlogic A1 SoC PLL controller support"
> > depends on ARM64
> > diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> > index 4968fc7ad555..2a06eb0303d6 100644
> > --- a/drivers/clk/meson/Makefile
> > +++ b/drivers/clk/meson/Makefile
> > @@ -18,6 +18,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_AUDIO_RSTC) += meson-audio-rstc.o
> >
> > obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
> > obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
> > +obj-$(CONFIG_COMMON_CLK_A1_CPU) += a1-cpu.o
> > obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
> > obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
> > obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
> > diff --git a/drivers/clk/meson/a1-cpu.c b/drivers/clk/meson/a1-cpu.c
> > new file mode 100644
> > index 000000000000..5f5d8ae112e5
> > --- /dev/null
> > +++ b/drivers/clk/meson/a1-cpu.c
> > @@ -0,0 +1,324 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Amlogic A1 SoC family CPU Clock Controller driver.
> > + *
> > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> > + * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/platform_device.h>
> > +#include "a1-cpu.h"
> > +#include "clk-regmap.h"
> > +#include "meson-clkc-utils.h"
> > +
> > +#include <dt-bindings/clock/amlogic,a1-cpu-clkc.h>
> > +
> > +static u32 cpu_fsource_sel_table[] = { 0, 1, 2 };
> > +static const struct clk_parent_data cpu_fsource_sel_parents[] = {
> > + { .fw_name = "xtal" },
> > + { .fw_name = "fclk_div2" },
> > + { .fw_name = "fclk_div3" },
> > +};
> > +
> > +static struct clk_regmap cpu_fsource_sel0 = {
> > + .data = &(struct clk_regmap_mux_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .mask = 0x3,
> > + .shift = 0,
> > + .table = cpu_fsource_sel_table,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fsource_sel0",
> > + .ops = &clk_regmap_mux_ops,
> > + .parent_data = cpu_fsource_sel_parents,
> > + .num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_fsource_div0 = {
> > + .data = &(struct clk_regmap_div_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .shift = 4,
> > + .width = 6,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fsource_div0",
> > + .ops = &clk_regmap_divider_ops,
> > + .parent_hws = (const struct clk_hw *[]) {
> > + &cpu_fsource_sel0.hw
> > + },
> > + .num_parents = 1,
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_fsel0 = {
> > + .data = &(struct clk_regmap_mux_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .mask = 0x1,
> > + .shift = 2,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fsel0",
> > + .ops = &clk_regmap_mux_ops,
> > + .parent_hws = (const struct clk_hw *[]) {
> > + &cpu_fsource_sel0.hw,
> > + &cpu_fsource_div0.hw,
> > + },
> > + .num_parents = 2,
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_fsource_sel1 = {
> > + .data = &(struct clk_regmap_mux_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .mask = 0x3,
> > + .shift = 16,
> > + .table = cpu_fsource_sel_table,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fsource_sel1",
> > + .ops = &clk_regmap_mux_ops,
> > + .parent_data = cpu_fsource_sel_parents,
> > + .num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_fsource_div1 = {
> > + .data = &(struct clk_regmap_div_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .shift = 20,
> > + .width = 6,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fsource_div1",
> > + .ops = &clk_regmap_divider_ops,
> > + .parent_hws = (const struct clk_hw *[]) {
> > + &cpu_fsource_sel1.hw
> > + },
> > + .num_parents = 1,
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_fsel1 = {
> > + .data = &(struct clk_regmap_mux_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .mask = 0x1,
> > + .shift = 18,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fsel1",
> > + .ops = &clk_regmap_mux_ops,
> > + .parent_hws = (const struct clk_hw *[]) {
> > + &cpu_fsource_sel1.hw,
> > + &cpu_fsource_div1.hw,
> > + },
> > + .num_parents = 2,
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_fclk = {
> > + .data = &(struct clk_regmap_mux_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .mask = 0x1,
> > + .shift = 10,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_fclk",
> > + .ops = &clk_regmap_mux_ops,
> > + .parent_hws = (const struct clk_hw *[]) {
> > + &cpu_fsel0.hw,
> > + &cpu_fsel1.hw,
> > + },
> > + .num_parents = 2,
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +static struct clk_regmap cpu_clk = {
> > + .data = &(struct clk_regmap_mux_data) {
> > + .offset = CPUCTRL_CLK_CTRL0,
> > + .mask = 0x1,
> > + .shift = 11,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "cpu_clk",
> > + .ops = &clk_regmap_mux_ops,
> > + .parent_data = (const struct clk_parent_data []) {
> > + { .hw = &cpu_fclk.hw },
> > + { .fw_name = "sys_pll", },
> > + },
> > + .num_parents = 2,
> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> > + },
> > +};
> > +
> > +/* Array of all clocks registered by this provider */
> > +static struct clk_hw *a1_cpu_hw_clks[] = {
> > + [CLKID_CPU_FSOURCE_SEL0] = &cpu_fsource_sel0.hw,
> > + [CLKID_CPU_FSOURCE_DIV0] = &cpu_fsource_div0.hw,
> > + [CLKID_CPU_FSEL0] = &cpu_fsel0.hw,
> > + [CLKID_CPU_FSOURCE_SEL1] = &cpu_fsource_sel1.hw,
> > + [CLKID_CPU_FSOURCE_DIV1] = &cpu_fsource_div1.hw,
> > + [CLKID_CPU_FSEL1] = &cpu_fsel1.hw,
> > + [CLKID_CPU_FCLK] = &cpu_fclk.hw,
> > + [CLKID_CPU_CLK] = &cpu_clk.hw,
> > +};
> > +
> > +static struct clk_regmap *const a1_cpu_regmaps[] = {
> > + &cpu_fsource_sel0,
> > + &cpu_fsource_div0,
> > + &cpu_fsel0,
> > + &cpu_fsource_sel1,
> > + &cpu_fsource_div1,
> > + &cpu_fsel1,
> > + &cpu_fclk,
> > + &cpu_clk,
> > +};
> > +
> > +static struct regmap_config a1_cpu_regmap_cfg = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = CPUCTRL_CLK_CTRL1,
> > +};
> > +
> > +static struct meson_clk_hw_data a1_cpu_clks = {
> > + .hws = a1_cpu_hw_clks,
> > + .num = ARRAY_SIZE(a1_cpu_hw_clks),
> > +};
> > +
> > +struct a1_cpu_clk_nb_data {
> > + const struct clk_ops *mux_ops;
>
> That's fishy ...
>
> > + struct clk_hw *cpu_clk;
> > + struct notifier_block nb;
> > + u8 parent;
> > +};
> > +
> > +#define MESON_A1_CPU_CLK_GET_PARENT(nbd) \
> > + ((nbd)->mux_ops->get_parent((nbd)->cpu_clk))
> > +#define MESON_A1_CPU_CLK_SET_PARENT(nbd, index) \
> > + ((nbd)->mux_ops->set_parent((nbd)->cpu_clk, index))
>
> ... Directly going for the mux ops ??!?? No way !
>
> We have a framework to handle the clocks, the whole point is to use it,
> not bypass it !
>
I suppose you understand my approach, which is quite similar to what is
happening in the Mediatek driver:
https://elixir.bootlin.com/linux/latest/source/drivers/clk/mediatek/clk-mux.c#L295
Initially, I attempted to set the parent using the clk_set_parent() API.
However, I encountered a problem with recursive calling of the
notifier_block. This issue arises because the parent triggers
notifications for its children, leading to repeated calls to the
notifier_block.
I find it puzzling why I cannot call an internal function or callback
within the internal driver context. After all, the notifier block is
just a part of the set_rate() flow. From a global Clock Control
Framework perspective, the context should not change.
> > +
> > +static int meson_a1_cpu_clk_notifier_cb(struct notifier_block *nb,
> > + unsigned long event, void *data)
> > +{
> > + struct a1_cpu_clk_nb_data *nbd;
> > + int ret = 0;
> > +
> > + nbd = container_of(nb, struct a1_cpu_clk_nb_data, nb);
> > +
> > + switch (event) {
> > + case PRE_RATE_CHANGE:
> > + nbd->parent = MESON_A1_CPU_CLK_GET_PARENT(nbd);
> > + /* Fallback to the CPU fixed clock */
> > + ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, 0);
> > + /* Wait for clock propagation */
> > + udelay(100);
> > + break;
> > +
> > + case POST_RATE_CHANGE:
> > + case ABORT_RATE_CHANGE:
> > + /* Back to the original parent clock */
> > + ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, nbd->parent);
> > + /* Wait for clock propagation */
> > + udelay(100);
> > + break;
> > +
> > + default:
> > + pr_warn("Unknown event %lu for %s notifier block\n",
> > + event, clk_hw_get_name(nbd->cpu_clk));
> > + break;
> > + }
> > +
> > + return notifier_from_errno(ret);
> > +}
> > +
> > +static struct a1_cpu_clk_nb_data a1_cpu_clk_nb_data = {
> > + .mux_ops = &clk_regmap_mux_ops,
> > + .cpu_clk = &cpu_clk.hw,
> > + .nb.notifier_call = meson_a1_cpu_clk_notifier_cb,
> > +};
> > +
> > +static int meson_a1_dvfs_setup(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct clk *notifier_clk;
> > + int ret;
> > +
> > + /* Setup clock notifier for cpu_clk */
> > + notifier_clk = devm_clk_hw_get_clk(dev, &cpu_clk.hw, "dvfs");
> > + if (IS_ERR(notifier_clk))
> > + return dev_err_probe(dev, PTR_ERR(notifier_clk),
> > + "can't get cpu_clk as notifier clock\n");
> > +
> > + ret = devm_clk_notifier_register(dev, notifier_clk,
> > + &a1_cpu_clk_nb_data.nb);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "can't register cpu_clk notifier\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int meson_a1_cpu_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + void __iomem *base;
> > + struct regmap *map;
> > + int clkid, i, err;
> > +
> > + base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(base))
> > + return dev_err_probe(dev, PTR_ERR(base),
> > + "can't ioremap resource\n");
> > +
> > + map = devm_regmap_init_mmio(dev, base, &a1_cpu_regmap_cfg);
> > + if (IS_ERR(map))
> > + return dev_err_probe(dev, PTR_ERR(map),
> > + "can't init regmap mmio region\n");
> > +
> > + /* Populate regmap for the regmap backed clocks */
> > + for (i = 0; i < ARRAY_SIZE(a1_cpu_regmaps); i++)
> > + a1_cpu_regmaps[i]->map = map;
> > +
> > + for (clkid = 0; clkid < a1_cpu_clks.num; clkid++) {
> > + err = devm_clk_hw_register(dev, a1_cpu_clks.hws[clkid]);
> > + if (err)
> > + return dev_err_probe(dev, err,
> > + "clock[%d] registration failed\n",
> > + clkid);
> > + }
> > +
> > + err = devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, &a1_cpu_clks);
> > + if (err)
> > + return dev_err_probe(dev, err, "can't add clk hw provider\n");
>
> I wonder if there is a window of opportunity to poke the syspll without
> your notifier here. That being said, the situation would be similar on g12.
>
Yes, I have taken into account what you did in the G12A CPU clock
relations. My thoughts were that it might not be applicable for the A1
case. This is because the sys_pll should be located in a different
driver from a logical perspective. Consequently, we cannot configure the
sys_pll notifier block to manage the cpu_clk from a different driver.
However, if I were to move the sys_pll clock object to the A1 CPU clock
controller, I believe the g12a sys_pll notifier approach would work.
> > +
> > + return meson_a1_dvfs_setup(pdev);
>
>
>
> > +}
> > +
> > +static const struct of_device_id a1_cpu_clkc_match_table[] = {
> > + { .compatible = "amlogic,a1-cpu-clkc", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, a1_cpu_clkc_match_table);
> > +
> > +static struct platform_driver a1_cpu_clkc_driver = {
> > + .probe = meson_a1_cpu_probe,
> > + .driver = {
> > + .name = "a1-cpu-clkc",
> > + .of_match_table = a1_cpu_clkc_match_table,
> > + },
> > +};
> > +
> > +module_platform_driver(a1_cpu_clkc_driver);
> > +MODULE_AUTHOR("Dmitry Rokosov <ddrokosov@salutedevices.com>");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/clk/meson/a1-cpu.h b/drivers/clk/meson/a1-cpu.h
> > new file mode 100644
> > index 000000000000..e9af4117e26f
> > --- /dev/null
> > +++ b/drivers/clk/meson/a1-cpu.h
>
> There is not point putting the definition here in a header
> These are clearly not going to be shared with another driver.
>
> Please drop this file
>
The same approach was applied to the Peripherals and PLL A1 drivers.
Honestly, I am not a fan of having different file organization within a
single logical code folder.
Please refer to:
drivers/clk/meson/a1-peripherals.h
drivers/clk/meson/a1-pll.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Amlogic A1 CPU Clock Controller internals
> > + *
> > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> > + * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
> > + */
> > +
> > +#ifndef __A1_CPU_H
> > +#define __A1_CPU_H
> > +
> > +/* cpu clock controller register offset */
> > +#define CPUCTRL_CLK_CTRL0 0x80
> > +#define CPUCTRL_CLK_CTRL1 0x84
>
> You are claiming the registers from 0x00 to 0x84 (included), but only
> using these 2 registers ? What is the rest ? Are you sure there is only
> clocks in there ?
>
Yes, unfortunately, the register map for this IP is not described in the
A1 Datasheet. The only available information about it can be found in
the vendor clock driver, which provides details for only two registers
used to configure the CPU clock.
From vendor kernel dtsi:
clkc: clock-controller {
compatible = "amlogic,a1-clkc";
#clock-cells = <1>;
reg = <0x0 0xfe000800 0x0 0x100>,
<0x0 0xfe007c00 0x0 0x21c>,
<0x0 0xfd000000 0x0 0x88>; <==== CPU clock regmap
reg-names = "basic", "pll",
"cpu_clk";
clocks = <&xtal>;
clock-names = "core";
status = "okay";
};
From vendor clkc driver:
/*
* CPU clok register offset
* APB_BASE: APB1_BASE_ADDR = 0xfd000000
*/
#define CPUCTRL_CLK_CTRL0 0x80
#define CPUCTRL_CLK_CTRL1 0x84
[...]
--
Thank you,
Dmitry
^ permalink raw reply
* Re: [PATCH v6 4/6] clk: qcom: common: Add interconnect clocks support
From: Varadarajan Narayanan @ 2024-04-02 11:05 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, quic_anusha,
linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <CAA8EJprP0m53B=g7jafAkfcqAQP4kE2ZvtxPXEe4s7ALjFXGSQ@mail.gmail.com>
On Tue, Apr 02, 2024 at 01:48:14PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > Unlike MSM platforms that manage NoC related clocks and scaling
> > from RPM, IPQ SoCs dont involve RPM in managing NoC related
> > clocks and there is no NoC scaling.
> >
> > However, there is a requirement to enable some NoC interface
> > clocks for accessing the peripheral controllers present on
> > these NoCs. Though exposing these as normal clocks would work,
> > having a minimalistic interconnect driver to handle these clocks
> > would make it consistent with other Qualcomm platforms resulting
> > in common code paths. This is similar to msm8996-cbf's usage of
> > icc-clk framework.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v6: first_id -> icc_first_node_id
> > Remove clock get so that the peripheral that uses the clock
> > can do the clock get
> > v5: Split changes in common.c to separate patch
> > Fix error handling
> > Use devm_icc_clk_register instead of icc_clk_register
> > v4: Use clk_hw instead of indices
> > Do icc register in qcom_cc_probe() call stream
> > Add icc clock info to qcom_cc_desc structure
> > v3: Use indexed identifiers here to avoid confusion
> > Fix error messages and move to common.c
> > v2: Move DTS to separate patch
> > Update commit log
> > Auto select CONFIG_INTERCONNECT & CONFIG_INTERCONNECT_CLK to fix build error
> > ---
> > drivers/clk/qcom/common.c | 38 +++++++++++++++++++++++++++++++++++++-
> > drivers/clk/qcom/common.h | 3 +++
> > 2 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> > index 75f09e6e057e..d5c008048994 100644
> > --- a/drivers/clk/qcom/common.c
> > +++ b/drivers/clk/qcom/common.c
> > @@ -8,6 +8,7 @@
> > #include <linux/regmap.h>
> > #include <linux/platform_device.h>
> > #include <linux/clk-provider.h>
> > +#include <linux/interconnect-clk.h>
> > #include <linux/reset-controller.h>
> > #include <linux/of.h>
> >
> > @@ -234,6 +235,41 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
> > return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
> > }
> >
> > +static int qcom_cc_icc_register(struct device *dev,
> > + const struct qcom_cc_desc *desc)
> > +{
> > + struct icc_clk_data *icd;
> > + int i;
> > +
> > + if (!IS_ENABLED(CONFIG_INTERCONNECT_CLK))
> > + return 0;
> > +
> > + if (!desc->icc_hws)
> > + return 0;
> > +
> > + icd = devm_kcalloc(dev, desc->num_icc_hws, sizeof(*icd), GFP_KERNEL);
> > + if (!icd)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < desc->num_icc_hws; i++) {
> > + /*
> > + * get_clk will be done by the peripheral device using this
> > + * clock with devm_clk_hw_get_clk() so that we can associate
> > + * the clk handle with the consumer device. It would also help
> > + * us make it so that drivers defer probe until their
> > + * clk isn't an orphan.
>
> How the clock instance returned to the peripheral driver is supposed
> to correspond to the clock instance used by the icc-clk?
> > + */
> > + icd[i].clk = desc->icc_hws[i]->clk;
>
> You again are abusing clk_hw->clk. Please don't do that.
Ok, will clk_get in both the places.
Thanks
Varada
> > + if (!icd[i].clk)
> > + return dev_err_probe(dev, -ENOENT,
> > + "(%d) clock entry is null\n", i);
> > + icd[i].name = clk_hw_get_name(desc->icc_hws[i]);
> > + }
> > +
> > + return PTR_ERR_OR_ZERO(devm_icc_clk_register(dev, desc->icc_first_node_id,
> > + desc->num_icc_hws, icd));
> > +}
> > +
> > int qcom_cc_really_probe(struct platform_device *pdev,
> > const struct qcom_cc_desc *desc, struct regmap *regmap)
> > {
> > @@ -303,7 +339,7 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> > if (ret)
> > return ret;
> >
> > - return 0;
> > + return qcom_cc_icc_register(dev, desc);
> > }
> > EXPORT_SYMBOL_GPL(qcom_cc_really_probe);
> >
> > diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> > index 9c8f7b798d9f..9058ffd46260 100644
> > --- a/drivers/clk/qcom/common.h
> > +++ b/drivers/clk/qcom/common.h
> > @@ -29,6 +29,9 @@ struct qcom_cc_desc {
> > size_t num_gdscs;
> > struct clk_hw **clk_hws;
> > size_t num_clk_hws;
> > + struct clk_hw **icc_hws;
> > + size_t num_icc_hws;
> > + unsigned int icc_first_node_id;
> > };
> >
> > /**
> > --
> > 2.34.1
> >
>
>
> --
> With best wishes
>
> Dmitry
^ permalink raw reply
* RE: [EXT] Re: [PATCH v10 03/11] arm64: dts: imx8ulp-evk: enable usb nodes and add ptn5150 nodes
From: Xu Yang @ 2024-04-02 11:06 UTC (permalink / raw)
To: Shawn Guo
Cc: gregkh@linuxfoundation.org, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
conor+dt@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
peter.chen@kernel.org, Jun Li, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
imx@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <ZgvDTPiBM65l3F+U@dragon>
>
> On Thu, Mar 21, 2024 at 04:14:31PM +0800, Xu Yang wrote:
> > Enable 2 USB nodes and add 2 PTN5150 nodes on i.MX8ULP evk board.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> > - fix format as suggusted by Fabio
> > - add PTN5150 nodes
> > Changes in v3:
> > - no changes
> > Changes in v4:
> > - no changes
> > Changes in v5:
> > - no changes
> > Changes in v6:
> > - no changes
> > Changes in v7:
> > - no changes
> > Changes in v8:
> > - no changes
> > Changes in v9:
> > - no changes
> > Changes in v10:
> > - no changes
> > ---
> > arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 84 +++++++++++++++++++
> > 1 file changed, 84 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> > index 69dd8e31027c..bf418af31039 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
> > @@ -133,6 +133,64 @@ pcal6408: gpio@21 {
> > gpio-controller;
> > #gpio-cells = <2>;
> > };
> > +
> > + ptn5150_1: typec@1d {
>
> Could you sort devices in unit-address?
Okay.
>
> > + compatible = "nxp,ptn5150";
> > + reg = <0x1d>;
> > + int-gpios = <&gpiof 3 IRQ_TYPE_EDGE_FALLING>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_typec1>;
> > + status = "disabled";
> > + };
> > +
> > + ptn5150_2: typec@3d {
> > + compatible = "nxp,ptn5150";
> > + reg = <0x3d>;
> > + int-gpios = <&gpiof 5 IRQ_TYPE_EDGE_FALLING>;
> > + pinctrl-names = "default";
>
> Broken indent?
Yes, will fix it.
Thanks,
Xu Yang
>
> Shawn
>
> > + pinctrl-0 = <&pinctrl_typec2>;
> > + status = "disabled";
> > + };
> > +};
> > +
> > +&usbotg1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_usb1>;
> > + dr_mode = "otg";
> > + hnp-disable;
> > + srp-disable;
> > + adp-disable;
> > + over-current-active-low;
> > + status = "okay";
> > +};
> > +
> > +&usbphy1 {
> > + fsl,tx-d-cal = <110>;
> > + status = "okay";
> > +};
> > +
> > +&usbmisc1 {
> > + status = "okay";
> > +};
> > +
> > +&usbotg2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_usb2>;
> > + dr_mode = "otg";
> > + hnp-disable;
> > + srp-disable;
> > + adp-disable;
> > + over-current-active-low;
> > + status = "okay";
> > +};
> > +
> > +&usbphy2 {
> > + fsl,tx-d-cal = <110>;
> > + status = "okay";
> > +};
> > +
> > +&usbmisc2 {
> > + status = "okay";
> > };
> >
> > &usdhc0 {
> > @@ -224,6 +282,32 @@ MX8ULP_PAD_PTE13__LPI2C7_SDA 0x20
> > >;
> > };
> >
> > + pinctrl_typec1: typec1grp {
> > + fsl,pins = <
> > + MX8ULP_PAD_PTF3__PTF3 0x3
> > + >;
> > + };
> > +
> > + pinctrl_typec2: typec2grp {
> > + fsl,pins = <
> > + MX8ULP_PAD_PTF5__PTF5 0x3
> > + >;
> > + };
> > +
> > + pinctrl_usb1: usb1grp {
> > + fsl,pins = <
> > + MX8ULP_PAD_PTF2__USB0_ID 0x10003
> > + MX8ULP_PAD_PTF4__USB0_OC 0x10003
> > + >;
> > + };
> > +
> > + pinctrl_usb2: usb2grp {
> > + fsl,pins = <
> > + MX8ULP_PAD_PTD23__USB1_ID 0x10003
> > + MX8ULP_PAD_PTF6__USB1_OC 0x10003
> > + >;
> > + };
> > +
> > pinctrl_usdhc0: usdhc0grp {
> > fsl,pins = <
> > MX8ULP_PAD_PTD1__SDHC0_CMD 0x3
> > --
> > 2.34.1
> >
^ permalink raw reply
* Re: [PATCH 5/5] arm64: dts: qcom: x1e80100: Enable cpufreq
From: Sudeep Holla @ 2024-04-02 11:09 UTC (permalink / raw)
To: Sibi Sankar
Cc: cristian.marussi, andersson, konrad.dybcio, jassisinghbrar,
robh+dt, krzysztof.kozlowski+dt, linux-kernel, linux-arm-msm,
devicetree, quic_rgottimu, quic_kshivnan, conor+dt, quic_gkohli,
quic_nkela, Ulf Hansson, quic_psodagud
In-Reply-To: <20240328095044.2926125-6-quic_sibis@quicinc.com>
On Thu, Mar 28, 2024 at 03:20:44PM +0530, Sibi Sankar wrote:
> Enable cpufreq on X1E80100 SoCs through the SCMI perf protocol node.
>
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/x1e80100.dtsi | 27 ++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 4e0ec859ed61..d1d232cd1f25 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -68,6 +68,7 @@ CPU0: cpu@0 {
> compatible = "qcom,oryon";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + clocks = <&scmi_dvfs 0>;
> next-level-cache = <&L2_0>;
> power-domains = <&CPU_PD0>;
> power-domain-names = "psci";
Any reason why you wouldn't want to use the new genpd based perf controls.
IIRC it was added based on mainly Qcom platform requirements.
- clocks = <&scmi_dvfs 0>;
next-level-cache = <&L2_0>;
- power-domains = <&CPU_PD0>;
- power-domain-names = "psci";
+ power-domains = <&CPU_PD0>, <&scmi_dvfs 0>;
+ power-domain-names = "psci", "perf";
And the associated changes in the scmi dvfs node for cells property.
This change is OK but just wanted to check the reasoning for the choice.
--
Regards,
Sudeep
^ permalink raw reply
* RE: [EXT] Re: [PATCH v10 09/11] arm64: dts: imx93-11x11-evk: enable usb and typec nodes
From: Xu Yang @ 2024-04-02 11:10 UTC (permalink / raw)
To: Shawn Guo
Cc: gregkh@linuxfoundation.org, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
conor+dt@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx,
peter.chen@kernel.org, Jun Li, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
imx@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <ZgvKteCEZJxShA/j@dragon>
>
> On Thu, Mar 21, 2024 at 04:14:37PM +0800, Xu Yang wrote:
> > There are 2 Type-C ports and 2 USB controllers on i.MX93. Enable them.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> > - remove status property in ptn5110 nodes
> > - fix dt-schema warnings
> > Changes in v3:
> > - no changes
> > Changes in v4:
> > - no changes
> > Changes in v5:
> > - no changes
> > Changes in v6:
> > - no changes
> > Changes in v7:
> > - no changes
> > Changes in v8:
> > - no changes
> > Changes in v9:
> > - use compatible "nxp,ptn5110", "tcpci"
> > Changes in v10:
> > - no changes
> > ---
> > .../boot/dts/freescale/imx93-11x11-evk.dts | 118 ++++++++++++++++++
> > 1 file changed, 118 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> > index 9921ea13ab48..ecc01d872e95 100644
> > --- a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> > @@ -5,6 +5,7 @@
> >
> > /dts-v1/;
> >
> > +#include <dt-bindings/usb/pd.h>
> > #include "imx93.dtsi"
> >
> > / {
> > @@ -104,6 +105,80 @@ &mu2 {
> > status = "okay";
> > };
> >
> > +&lpi2c3 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + clock-frequency = <400000>;
> > + pinctrl-names = "default", "sleep";
> > + pinctrl-0 = <&pinctrl_lpi2c3>;
> > + pinctrl-1 = <&pinctrl_lpi2c3>;
>
> Do you really need "sleep" pinctrl state?
"sleep" pinctrl state can be removed.
>
> > + status = "okay";
> > +
> > + ptn5110: tcpc@50 {
> > + compatible = "nxp,ptn5110", "tcpci";
> > + reg = <0x50>;
> > + interrupt-parent = <&gpio3>;
> > + interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> > +
> > + typec1_con: connector {
> > + compatible = "usb-c-connector";
> > + label = "USB-C";
> > + power-role = "dual";
> > + data-role = "dual";
> > + try-power-role = "sink";
> > + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> > + sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> > + PDO_VAR(5000, 20000, 3000)>;
> > + op-sink-microwatt = <15000000>;
> > + self-powered;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
>
> Have a newline between properties and child node.
Okay.
Thanks,
Xu Yang
>
> Shawn
>
> > + typec1_dr_sw: endpoint {
> > + remote-endpoint = <&usb1_drd_sw>;
> > + };
> > + };
> > + };
> > + };
> > + };
> > +
> > + ptn5110_2: tcpc@51 {
> > + compatible = "nxp,ptn5110", "tcpci";
> > + reg = <0x51>;
> > + interrupt-parent = <&gpio3>;
> > + interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> > +
> > + typec2_con: connector {
> > + compatible = "usb-c-connector";
> > + label = "USB-C";
> > + power-role = "dual";
> > + data-role = "dual";
> > + try-power-role = "sink";
> > + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
> > + sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)
> > + PDO_VAR(5000, 20000, 3000)>;
> > + op-sink-microwatt = <15000000>;
> > + self-powered;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + typec2_dr_sw: endpoint {
> > + remote-endpoint = <&usb2_drd_sw>;
> > + };
> > + };
> > + };
> > + };
> > + };
> > +};
> > +
> > &eqos {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_eqos>;
> > @@ -156,6 +231,42 @@ &lpuart5 {
> > status = "okay";
> > };
> >
> > +&usbotg1 {
> > + dr_mode = "otg";
> > + hnp-disable;
> > + srp-disable;
> > + adp-disable;
> > + usb-role-switch;
> > + disable-over-current;
> > + samsung,picophy-pre-emp-curr-control = <3>;
> > + samsung,picophy-dc-vol-level-adjust = <7>;
> > + status = "okay";
> > +
> > + port {
> > + usb1_drd_sw: endpoint {
> > + remote-endpoint = <&typec1_dr_sw>;
> > + };
> > + };
> > +};
> > +
> > +&usbotg2 {
> > + dr_mode = "otg";
> > + hnp-disable;
> > + srp-disable;
> > + adp-disable;
> > + usb-role-switch;
> > + disable-over-current;
> > + samsung,picophy-pre-emp-curr-control = <3>;
> > + samsung,picophy-dc-vol-level-adjust = <7>;
> > + status = "okay";
> > +
> > + port {
> > + usb2_drd_sw: endpoint {
> > + remote-endpoint = <&typec2_dr_sw>;
> > + };
> > + };
> > +};
> > +
> > &usdhc1 {
> > pinctrl-names = "default", "state_100mhz", "state_200mhz";
> > pinctrl-0 = <&pinctrl_usdhc1>;
> > @@ -222,6 +333,13 @@ MX93_PAD_ENET2_TX_CTL__ENET1_RGMII_TX_CTL 0x57e
> > >;
> > };
> >
> > + pinctrl_lpi2c3: lpi2c3grp {
> > + fsl,pins = <
> > + MX93_PAD_GPIO_IO28__LPI2C3_SDA 0x40000b9e
> > + MX93_PAD_GPIO_IO29__LPI2C3_SCL 0x40000b9e
> > + >;
> > + };
> > +
> > pinctrl_uart1: uart1grp {
> > fsl,pins = <
> > MX93_PAD_UART1_RXD__LPUART1_RX 0x31e
> > --
> > 2.34.1
> >
^ permalink raw reply
* Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
From: Dmitry Baryshkov @ 2024-04-02 11:16 UTC (permalink / raw)
To: Varadarajan Narayanan
Cc: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, quic_anusha,
linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <ZgvlrbvvPNA6HRiL@hu-varada-blr.qualcomm.com>
On Tue, 2 Apr 2024 at 14:02, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> > On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > > <quic_varada@quicinc.com> wrote:
> > > >
> > > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > > able to release the resources properly.
> > > >
> > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > ---
> > > > v5: Introduced devm_icc_clk_register
> > > > ---
> > > > drivers/interconnect/icc-clk.c | 29 +++++++++++++++++++++++++++++
> > > > include/linux/interconnect-clk.h | 4 ++++
> > > > 2 files changed, 33 insertions(+)
> > >
> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Wait. Actually,
> >
> > Unreviewed-by: me
> >
> > Please return int from devm_icc_clk_register instead of returning the pointer.
>
> Wouldn't returning int break the general assumption that
> devm_foo(), returns the same type as foo(). For example
> devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?
Not always. The only reason to return icc_provider was to make it
possible to destroy it. With devres-managed function you don't have to
do anything.
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH v3 0/2] mfd: rohm-bd71828: Add power off
From: Andreas Kemnade @ 2024-04-02 11:16 UTC (permalink / raw)
To: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
devicetree, linux-kernel
Cc: Andreas Kemnade
Add power off functionality. Marked as RFC because of magic numbers
without a good source and strange delays. The only information source is
a vendor kernel.
Changes in v3:
- define for poweroff bit
- rmw operation to set only that bit
Changes in v2:
- style corrections
- remove unnecessary writes and delays
- correctly unregister handler
Andreas Kemnade (2):
dt-bindings: mfd: Add ROHM BD71828 system-power-controller property
mfd: rohm-bd71828: Add power off functionality
.../bindings/mfd/rohm,bd71828-pmic.yaml | 2 ++
drivers/mfd/rohm-bd71828.c | 36 ++++++++++++++++++-
include/linux/mfd/rohm-bd71828.h | 3 ++
3 files changed, 40 insertions(+), 1 deletion(-)
--
2.39.2
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: mfd: Add ROHM BD71828 system-power-controller property
From: Andreas Kemnade @ 2024-04-02 11:16 UTC (permalink / raw)
To: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
devicetree, linux-kernel
Cc: Andreas Kemnade, Krzysztof Kozlowski
In-Reply-To: <20240402111700.494004-1-andreas@kemnade.info>
As the PMIC can power off the system, add the corresponding property.
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
index 11089aa89ec6..0b62f854bf6b 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml
@@ -73,6 +73,8 @@ properties:
used to mark the pins which should not be configured for GPIO. Please see
the ../gpio/gpio.txt for more information.
+ system-power-controller: true
+
required:
- compatible
- reg
--
2.39.2
^ permalink raw reply related
* [PATCH v3 2/2] mfd: rohm-bd71828: Add power off functionality
From: Andreas Kemnade @ 2024-04-02 11:17 UTC (permalink / raw)
To: lee, robh+dt, krzysztof.kozlowski+dt, conor+dt, mazziesaccount,
devicetree, linux-kernel
Cc: Andreas Kemnade
In-Reply-To: <20240402111700.494004-1-andreas@kemnade.info>
Since the chip can power off the system, add the corresponding
functionality.
Based on https://github.com/kobolabs/Kobo-Reader/raw/master/hw/imx6sll-clara2e/kernel.tar.bz2
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
drivers/mfd/rohm-bd71828.c | 36 +++++++++++++++++++++++++++++++-
include/linux/mfd/rohm-bd71828.h | 3 +++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index 594718f7e8e1..4a1fa8a0d76a 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -464,6 +464,27 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
OUT32K_MODE_CMOS);
}
+static struct i2c_client *bd71828_dev;
+static void bd71828_power_off(void)
+{
+ while (true) {
+ s32 val;
+
+ /* We are not allowed to sleep, so do not use regmap involving mutexes here. */
+ val = i2c_smbus_read_byte_data(bd71828_dev, BD71828_REG_PS_CTRL_1);
+ if (val >= 0)
+ i2c_smbus_write_byte_data(bd71828_dev,
+ BD71828_REG_PS_CTRL_1,
+ BD71828_MASK_STATE_HBNT | (u8)val);
+ mdelay(500);
+ }
+}
+
+static void bd71828_remove_poweroff(void *data)
+{
+ pm_power_off = NULL;
+}
+
static int bd71828_i2c_probe(struct i2c_client *i2c)
{
struct regmap_irq_chip_data *irq_data;
@@ -542,7 +563,20 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
NULL, 0, regmap_irq_get_domain(irq_data));
if (ret)
- dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+ return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+
+ if (of_device_is_system_power_controller(i2c->dev.of_node) &&
+ chip_type == ROHM_CHIP_TYPE_BD71828) {
+ if (!pm_power_off) {
+ bd71828_dev = i2c;
+ pm_power_off = bd71828_power_off;
+ ret = devm_add_action_or_reset(&i2c->dev,
+ bd71828_remove_poweroff,
+ NULL);
+ } else {
+ dev_warn(&i2c->dev, "Poweroff callback already assigned\n");
+ }
+ }
return ret;
}
diff --git a/include/linux/mfd/rohm-bd71828.h b/include/linux/mfd/rohm-bd71828.h
index 3b5f3a7db4bd..9776fde1262d 100644
--- a/include/linux/mfd/rohm-bd71828.h
+++ b/include/linux/mfd/rohm-bd71828.h
@@ -4,6 +4,7 @@
#ifndef __LINUX_MFD_BD71828_H__
#define __LINUX_MFD_BD71828_H__
+#include <linux/bits.h>
#include <linux/mfd/rohm-generic.h>
#include <linux/mfd/rohm-shared.h>
@@ -41,6 +42,8 @@ enum {
#define BD71828_REG_PS_CTRL_2 0x05
#define BD71828_REG_PS_CTRL_3 0x06
+#define BD71828_MASK_STATE_HBNT BIT(1)
+
//#define BD71828_REG_SWRESET 0x06
#define BD71828_MASK_RUN_LVL_CTRL 0x30
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
From: Varadarajan Narayanan @ 2024-04-02 11:22 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: andersson, konrad.dybcio, mturquette, sboyd, robh,
krzysztof.kozlowski+dt, conor+dt, djakov, quic_anusha,
linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <CAA8EJpp2dgy0DcLoUuo6gz-8ee0RRwJ_mvCLGDbdvF-gVhREFg@mail.gmail.com>
On Tue, Apr 02, 2024 at 02:16:56PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 14:02, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> > > On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> > > <dmitry.baryshkov@linaro.org> wrote:
> > > >
> > > > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > > > <quic_varada@quicinc.com> wrote:
> > > > >
> > > > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > > > able to release the resources properly.
> > > > >
> > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > ---
> > > > > v5: Introduced devm_icc_clk_register
> > > > > ---
> > > > > drivers/interconnect/icc-clk.c | 29 +++++++++++++++++++++++++++++
> > > > > include/linux/interconnect-clk.h | 4 ++++
> > > > > 2 files changed, 33 insertions(+)
> > > >
> > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >
> > > Wait. Actually,
> > >
> > > Unreviewed-by: me
> > >
> > > Please return int from devm_icc_clk_register instead of returning the pointer.
> >
> > Wouldn't returning int break the general assumption that
> > devm_foo(), returns the same type as foo(). For example
> > devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?
>
> Not always. The only reason to return icc_provider was to make it
> possible to destroy it. With devres-managed function you don't have to
> do anything.
Ok. Will change as follows
return prov; -> return PTR_ERR_OR_ZERO(prov);
Thanks
Varada
^ permalink raw reply
* Re: [PATCH 1/2] ARM: boot: dts: microchip: at91-sama7g5ek: Replace regulator-suspend-voltage with the valid property
From: Andrei.Simion @ 2024-04-02 11:27 UTC (permalink / raw)
To: krzysztof.kozlowski, robh, krzysztof.kozlowski+dt, conor+dt,
Nicolas.Ferre, alexandre.belloni, claudiu.beznea, Mihai.Sain
Cc: linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <34543ae2-4e78-45a4-9cff-389f7495fd4a@linaro.org>
On 02.04.2024 13:39, Krzysztof Kozlowski wrote:
> [You don't often get email from krzysztof.kozlowski@linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 02/04/2024 11:12, Andrei Simion wrote:
>> Replace regulator-suspend-voltage with regulator-suspend-microvolt.
>
> Why?
>
at91-sama7g5ek.dtb: mcp16502@5b: regulators:VDD_(CORE|OTHER)|LDO[1-2]:
regulator-state-standby 'regulator-suspend-voltage' does not match any of
the regexes 'pinctrl-[0-9]+' from schema
$id: http://devicetree.org/schemas/regulator/microchip,mcp16502.yaml#
no property named regulator-suspend-voltage in
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/regulator/regulator.yaml
so if it is using this property there will be no effect as it was expected in
https://github.com/torvalds/linux/commit/85b1304b9daa06367139b471789c7ddb76250b9f
> Please explain what is the bug and how it manifests itself. Is one
> property incorrect and other correct?
>
The main reason is explained in the cover-letter but if you ask me to explain in each commit I will do it in next version.
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
> Hint: there is no "boot"
>
Yes, in hurry I slipped that "boot" in subject.
>>
>
> Best regards,
> Krzysztof
>
--
Andrei Simion
^ permalink raw reply
* Re: [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock controller driver
From: Dmitry Rokosov @ 2024-04-02 11:43 UTC (permalink / raw)
To: Jerome Brunet
Cc: Martin Blumenstingl, neil.armstrong, mturquette, sboyd, robh+dt,
krzysztof.kozlowski+dt, khilman, kernel, rockosov, linux-amlogic,
linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1jzfuchztl.fsf@starbuckisacylon.baylibre.com>
On Tue, Apr 02, 2024 at 11:27:24AM +0200, Jerome Brunet wrote:
>
> On Mon 01 Apr 2024 at 20:12, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
>
> > Hello Martin,
> >
> > Thank you for quick response. Please find my thoughts below.
> >
> > On Sun, Mar 31, 2024 at 11:40:13PM +0200, Martin Blumenstingl wrote:
> >> Hi Dmitry,
> >>
> >> On Fri, Mar 29, 2024 at 9:59 PM Dmitry Rokosov
> >> <ddrokosov@salutedevices.com> wrote:
> >> [...]
> >> > +static struct clk_regmap cpu_fclk = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x1,
> >> > + .shift = 10,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fclk",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_hws = (const struct clk_hw *[]) {
> >> > + &cpu_fsel0.hw,
> >> > + &cpu_fsel1.hw,
> >> Have you considered the CLK_SET_RATE_GATE flag for &cpu_fsel0.hw and
> >> &cpu_fsel1.hw and then dropping the clock notifier below?
> >> We use that approach with the Mali GPU clock on other SoCs, see for
> >> example commit 8daeaea99caa ("clk: meson: meson8b: make the CCF use
> >> the glitch-free mali mux").
> >> It may differ from what Amlogic does in their BSP,
> >
> > Amlogic in their BSP takes a different approach, which is slightly
> > different from mine. They cleverly change the parent of cpu_clk directly
> > by forking the cpufreq driver to a custom version. I must admit, it's
> > quite an "interesting and amazing" idea :) but it's not architecturally
> > correct totally.
>
> I disagree. Martin's suggestion is correct for the fsel part which is
> symetric.
>
It seems that I didn't fully understand Martin's suggestion. I was
confused by the advice to remove the notifier block and tried to explain
why it's not possible. However, I believe it is reasonable to protect
the cpu_fselX from rate propagation, because it is symmetric, as you
mentioned.
> >
> >> but I don't think
> >> that there's any harm (if it works in general) because CCF (common
> >> clock framework) will set all clocks in the "inactive" tree and then
> >> as a last step just change the mux (&cpu_fclk.hw). So at no point in
> >> time will we get any other rate than a) the original CPU clock rate
> >> before the rate change b) the new desired CPU clock rate. This is
> >> because we have two symmetric clock trees.
> >
> > Now, let's dive into the specifics of the issue we're facing. I've
> > examined the CLK_SET_RATE_GATE flag, which, to my understanding, blocks
> > rate changes for the entire clock chain. However, in this particular
> > situation, it doesn't provide the solution we need.
> >
> > Here's the problem we're dealing with:
> >
> > 1) The CPU clock can have the following frequency points:
> >
> > available frequency steps: 128 MHz, 256 MHz, 512 MHz, 768 MHz, 1.01 GHz, 1.20 GHz
> >
> > When we run the cpupower, we get the following information:
> > # cpupower -c 0,1 frequency-info
> > analyzing CPU 0:
> > driver: cpufreq-dt
> > CPUs which run at the same hardware frequency: 0 1
> > CPUs which need to have their frequency coordinated by software: 0 1
> > maximum transition latency: 50.0 us
> > hardware limits: 128 MHz - 1.20 GHz
> > available frequency steps: 128 MHz, 256 MHz, 512 MHz, 768 MHz, 1.01 GHz, 1.20 GHz
> > available cpufreq governors: conservative ondemand userspace performance schedutil
> > current policy: frequency should be within 128 MHz and 128 MHz.
> > The governor "schedutil" may decide which speed to use
> > within this range.
> > current CPU frequency: 128 MHz (asserted by call to hardware)
> > analyzing CPU 1:
> > driver: cpufreq-dt
> > CPUs which run at the same hardware frequency: 0 1
> > CPUs which need to have their frequency coordinated by software: 0 1
> > maximum transition latency: 50.0 us
> > hardware limits: 128 MHz - 1.20 GHz
> > available frequency steps: 128 MHz, 256 MHz, 512 MHz, 768 MHz, 1.01 GHz, 1.20 GHz
> > available cpufreq governors: conservative ondemand userspace performance schedutil
> > current policy: frequency should be within 128 MHz and 128 MHz.
> > The governor "schedutil" may decide which speed to use
> > within this range.
> > current CPU frequency: 128 MHz (asserted by call to hardware)
> >
> > 2) For the frequency points 128 MHz, 256 MHz, and 512 MHz, the CPU fixed
> > clock should be used.
>
> Apparently, you are relying on the SYS PLL lowest possible rate to
> enfore this contraint, which I suppose is 24 * 32 = 768MHz. It would be
> nice to clearly say so.
>
Based on my understanding, the minimum frequency that sys_pll can
provide is not relevant. The CPU fixed clock is considered a "safety"
clock, and I can confidently connect the cpu_clk parent to that stable
clock without any issues. CCF will decide which parent will be used in
the end of rate changing process.
> > Fortunately, we don't encounter any freeze
> > problems when we attempt to change its rate at these frequencies.
>
> That does not sound very solid ...
>
Why? Per my understanding, CPU fixed clock guarantees this behaviour.
> >
> > 3) However, for the frequency points 768 MHz, 1.01 GHz, and 1.20 GHz,
> > the sys_pll is used as the clock source because it's a faster option.
> > Now, let's imagine that we want to change the CPU clock from 768 MHz to
> > 1.01 GHz. Unfortunately, it's not possible due to the broken sys_pll,
> > and any execution attempts will result in a hang.
>
> ... Because PLL needs to relock, it is going to be off for a while. That
> is not "broken", unless there is something else ?
>
Sorry for wrong terminology. I meant that sys_pll cannot be used as a
clock source (clock parent) while we are changing its rate.
> >
> > 4) As you can observe, in this case, we actually don't need to lock the
> > rate for the sys_pll chain.
>
> In which case ? I'm lost.
>
In the case for which notifier block was applied - cpu_clk and sys_pll
rate propagation.
> > We want to change the rate instead.
>
> ... How are you going to do that without relocking the PLL ?
>
I'm afraid, this is terminology miss from my side again. By 'sys_pll
lock' I mean rate lock using CLK_SET_RATE_GATE. I want to say that we
can't prohibit rate propagation of sys_pll chain, because we want to
change the rate, this is our main goal.
> > Hence,
> > I'm not aware of any other method to achieve this except by switching
> > the cpu_clk parent to a stable clock using clock notifier block.
> > Interestingly, I've noticed a similar approach in the CPU clock drivers
> > of Rockchip, Qualcomm, and Mediatek.
>
> There is an example of syspll notifier in the g12 clock controller.
> You should have a look at it
Okay. As I mentioned in another email reply, in order to make it happen,
it is required to move the sys_pll clock to the a1-cpu driver. However,
I thought that this approach may not be correct from a logical
perspective. I will try.
--
Thank you,
Dmitry
^ permalink raw reply
* [PATCH] dt-bindings: watchdog: Convert Aspeed binding to DT schema
From: Andrew Jeffery @ 2024-04-02 12:01 UTC (permalink / raw)
To: wim, linux
Cc: Andrew Jeffery, robh, krzysztof.kozlowski+dt, conor+dt, joel, zev,
linux-watchdog, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
Squash warnings such as:
```
arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/apb@1e600000/watchdog@1e785000: failed to match any schema with compatible: ['aspeed,ast2400-wdt']
```
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
.../bindings/watchdog/aspeed,ast2400-wdt.yaml | 130 ++++++++++++++++++
.../bindings/watchdog/aspeed-wdt.txt | 73 ----------
2 files changed, 130 insertions(+), 73 deletions(-)
create mode 100644 Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
delete mode 100644 Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
new file mode 100644
index 000000000000..10fcb50c4051
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/aspeed,ast2400-wdt.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/aspeed,ast2400-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed watchdog timer controllers
+
+maintainers:
+ - Andrew Jeffery <andrew@codeconstruct.com.au>
+
+properties:
+ compatible:
+ enum:
+ - aspeed,ast2400-wdt
+ - aspeed,ast2500-wdt
+ - aspeed,ast2600-wdt
+
+ reg:
+ maxItems: 1
+
+ clocks: true
+
+ aspeed,reset-type:
+ enum:
+ - cpu
+ - soc
+ - system
+ - none
+ description: |
+ Reset behaviour - The watchdog can be programmed to generate one of three
+ different types of reset when a timeout occcurs.
+
+ Specifying 'cpu' will only reset the processor on a timeout event.
+
+ Specifying 'soc' will reset a configurable subset of the SoC's controllers
+ on a timeout event. Controllers critical to the SoC's operation may remain untouched.
+
+ Specifying 'system' will reset all controllers on a timeout event, as if EXTRST had been asserted.
+ Specifying "none" will cause the timeout event to have no reset effect.
+ Another watchdog engine on the chip must be used for chip reset operations.
+
+ The default reset type is "system"
+
+ aspeed,alt-boot:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ Direct the watchdog to configure the SoC to boot from the alternative boot
+ region if a timeout occurs.
+
+ aspeed,external-signal:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ Assert the timeout event on an external signal pin associated with the
+ watchdog controller instance. The pin must be muxed appropriately.
+
+ aspeed,ext-pulse-duration:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ The duration, in microseconds, of the pulse emitted on the external signal pin
+
+ aspeed,ext-push-pull:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ If aspeed,external-signal is specified in the node, set the external
+ signal pin's drive type to push-pull. If aspeed,ext-push-pull is not
+ specified then the pin is configured as open-drain.
+
+ aspeed,ext-active-high:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ If both aspeed,external-signal and aspeed,ext-push-pull are specified in
+ the node, set the pulse polarity to active-high. If aspeed,ext-active-high
+ is not specified then the pin is configured as active-low.
+
+ aspeed,reset-mask:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 2
+ description: |
+ A bitmaks indicating which peripherals will be reset if the watchdog
+ timer expires. On AST2500 SoCs this should be a single word defined using
+ the AST2500_WDT_RESET_* macros; on AST2600 SoCs this should be a two-word
+ array with the first word defined using the AST2600_WDT_RESET1_* macros,
+ and the second word defined using the AST2600_WDT_RESET2_* macros.
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - if:
+ anyOf:
+ - required:
+ - aspeed,ext-push-pull
+ - required:
+ - aspeed,ext-active-high
+ - required:
+ - aspeed,reset-mask
+ then:
+ properties:
+ compatible:
+ enum:
+ - aspeed,ast2500-wdt
+ - aspeed,ast2600-wdt
+ - if:
+ required:
+ - aspeed,ext-active-high
+ then:
+ required:
+ - aspeed,ext-push-pull
+
+additionalProperties: false
+
+examples:
+ - |
+ wdt1: watchdog@1e785000 {
+ compatible = "aspeed,ast2400-wdt";
+ reg = <0x1e785000 0x1c>;
+ aspeed,reset-type = "system";
+ aspeed,external-signal;
+ };
+ - |
+ #include <dt-bindings/watchdog/aspeed-wdt.h>
+ wdt2: watchdog@1e785040 {
+ compatible = "aspeed,ast2600-wdt";
+ reg = <0x1e785040 0x40>;
+ aspeed,reset-mask = <AST2600_WDT_RESET1_DEFAULT
+ (AST2600_WDT_RESET2_DEFAULT & ~AST2600_WDT_RESET2_LPC)>;
+ };
diff --git a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt b/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
deleted file mode 100644
index 3208adb3e52e..000000000000
--- a/Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-Aspeed Watchdog Timer
-
-Required properties:
- - compatible: must be one of:
- - "aspeed,ast2400-wdt"
- - "aspeed,ast2500-wdt"
- - "aspeed,ast2600-wdt"
-
- - reg: physical base address of the controller and length of memory mapped
- region
-
-Optional properties:
-
- - aspeed,reset-type = "cpu|soc|system|none"
-
- Reset behavior - Whenever a timeout occurs the watchdog can be programmed
- to generate one of three different, mutually exclusive, types of resets.
-
- Type "none" can be specified to indicate that no resets are to be done.
- This is useful in situations where another watchdog engine on chip is
- to perform the reset.
-
- If 'aspeed,reset-type=' is not specified the default is to enable system
- reset.
-
- Reset types:
-
- - cpu: Reset CPU on watchdog timeout
-
- - soc: Reset 'System on Chip' on watchdog timeout
-
- - system: Reset system on watchdog timeout
-
- - none: No reset is performed on timeout. Assumes another watchdog
- engine is responsible for this.
-
- - aspeed,alt-boot: If property is present then boot from alternate block.
- - aspeed,external-signal: If property is present then signal is sent to
- external reset counter (only WDT1 and WDT2). If not
- specified no external signal is sent.
- - aspeed,ext-pulse-duration: External signal pulse duration in microseconds
-
-Optional properties for AST2500-compatible watchdogs:
- - aspeed,ext-push-pull: If aspeed,external-signal is present, set the pin's
- drive type to push-pull. The default is open-drain.
- - aspeed,ext-active-high: If aspeed,external-signal is present and and the pin
- is configured as push-pull, then set the pulse
- polarity to active-high. The default is active-low.
-
-Optional properties for AST2500- and AST2600-compatible watchdogs:
- - aspeed,reset-mask: A bitmask indicating which peripherals will be reset if
- the watchdog timer expires. On AST2500 this should be a
- single word defined using the AST2500_WDT_RESET_* macros;
- on AST2600 this should be a two-word array with the first
- word defined using the AST2600_WDT_RESET1_* macros and the
- second word defined using the AST2600_WDT_RESET2_* macros.
-
-Examples:
-
- wdt1: watchdog@1e785000 {
- compatible = "aspeed,ast2400-wdt";
- reg = <0x1e785000 0x1c>;
- aspeed,reset-type = "system";
- aspeed,external-signal;
- };
-
- #include <dt-bindings/watchdog/aspeed-wdt.h>
- wdt2: watchdog@1e785040 {
- compatible = "aspeed,ast2600-wdt";
- reg = <0x1e785040 0x40>;
- aspeed,reset-mask = <AST2600_WDT_RESET1_DEFAULT
- (AST2600_WDT_RESET2_DEFAULT & ~AST2600_WDT_RESET2_LPC)>;
- };
--
2.39.2
^ permalink raw reply related
* Re: [PATCH 1/2] ARM: boot: dts: microchip: at91-sama7g5ek: Replace regulator-suspend-voltage with the valid property
From: Krzysztof Kozlowski @ 2024-04-02 12:07 UTC (permalink / raw)
To: Andrei.Simion, robh, krzysztof.kozlowski+dt, conor+dt,
Nicolas.Ferre, alexandre.belloni, claudiu.beznea, Mihai.Sain
Cc: linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <312bed1a-0b8a-457d-a2e2-b8ee1b6f443b@microchip.com>
On 02/04/2024 13:27, Andrei.Simion@microchip.com wrote:
> On 02.04.2024 13:39, Krzysztof Kozlowski wrote:
>> [You don't often get email from krzysztof.kozlowski@linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 02/04/2024 11:12, Andrei Simion wrote:
>>> Replace regulator-suspend-voltage with regulator-suspend-microvolt.
>>
>> Why?
>>
>
> at91-sama7g5ek.dtb: mcp16502@5b: regulators:VDD_(CORE|OTHER)|LDO[1-2]:
> regulator-state-standby 'regulator-suspend-voltage' does not match any of
> the regexes 'pinctrl-[0-9]+' from schema
> $id: http://devicetree.org/schemas/regulator/microchip,mcp16502.yaml#
>
> no property named regulator-suspend-voltage in
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/regulator/regulator.yaml
> so if it is using this property there will be no effect as it was expected in
> https://github.com/torvalds/linux/commit/85b1304b9daa06367139b471789c7ddb76250b9f
>
>> Please explain what is the bug and how it manifests itself. Is one
>> property incorrect and other correct?
>>
> The main reason is explained in the cover-letter but if you ask me to explain in each commit I will do it in next version.
Cover letter does not go to commit history. Each commit should explain
why you are doing it. Usually piece of the warning is quite
self-explanatory, thus one easy way to achieve the point - answer why.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v8 0/7] spmi: pmic-arb: Add support for multiple buses
From: Abel Vesa @ 2024-04-02 12:07 UTC (permalink / raw)
To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Abel Vesa,
Krzysztof Kozlowski
This patchset prepares for and adds support for 2 buses, which is supported
in HW starting with version 7. Until now, none of the currently
supported platforms in upstream have used the second bus. The X1E80100
platform, on the other hand, needs the second bus for the USB2.0 to work
as there are 3 SMB2360 PMICs which provide eUSB2 repeaters and they are
all found on the second bus.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Changes in v8:
- Added Neil's R-b tag to the 3rd patch
- Fixed compile warnings already existent by adding another patch
- Fixed compile warning about get_core_resources, reported by Neil
- Dropped and moved the spurious core removal changes, as suggested by Neil
- Link to v7: https://lore.kernel.org/r/20240329-spmi-multi-master-support-v7-0-7b902824246c@linaro.org
Changes in v7:
- This time really collected Krzysztof's R-b tags
- Added Neil's R-b tag to the 4th patch
- Split the multi bus patch into two separate patches, one for adding
the bus object and one for the secondary bus, as per Neil's suggestion
- Fixed regression for single bus platforms triggered by casting to
pmic_arb instead of bus in pmic_arb_non_data_cmd_v1
- Fixed bus object allocation by using ctrl drvdata instead
- Prefixed the spmi node property in x1e80100 schema with '^'
- Fixed struct and function documentation warnings reported by Neil
Changes in v6 (resend):
- Collected Krzysztof's R-b tags
- Link to v6: https://lore.kernel.org/r/20240222-spmi-multi-master-support-v6-0-bc34ea9561da@linaro.org
Changes in v6:
- Changed the compatible to platform specific (X1E80100) along with the
schema. Fixed the spmi buses unit addresses and added the empty ranges
property. Added missing properties to the spmi buses and the
"unevaluatedProperties: false".
- Deprecated the "qcom,bus-id" in the legacy schema.
- Changed the driver to check for legacy compatible first
- Link to v5: https://lore.kernel.org/r/20240221-spmi-multi-master-support-v5-0-3255ca413a0b@linaro.org
Changes in v5:
- Dropped the RFC as there aren't any concerns about the approach anymore
- Dropped the unused dev and res variables from pmic_arb_get_obsrvr_chnls_v2
- Link to v4: https://lore.kernel.org/r/20240220-spmi-multi-master-support-v4-0-dc813c878ba8@linaro.org
Changes in v4:
- Fixed comment above pmic_arb_init_apid_v7 by dropping the extra "bus" word
- Swicthed to devm_platform_ioremap_resource_byname for obsrvr and chnls.
The core remains with platform_get_resource_byname as we need the core size.
- Dropped comment from probe related to the need of platform_get_resource_byname
as it not true anymore.
- Dropped the qcom,bus-id optional property.
- Link to v3: https://lore.kernel.org/r/20240214-spmi-multi-master-support-v3-0-0bae0ef04faf@linaro.org
Changes in v3:
- Split the change into 3 separate patches. First 2 patches are moving
apid init and core resources into version specific ops. Third one is
adding the support for 2 buses and dedicated compatible.
- Added separate bindings patch
- Link to v2: https://lore.kernel.org/r/20240213-spmi-multi-master-support-v2-1-b3b102326906@linaro.org
Changes in v2:
- Reworked it so that it registers a spmi controller for each bus
rather than relying on the generic framework to pass on the bus
(master) id.
- Link to v1: https://lore.kernel.org/r/20240207-spmi-multi-master-support-v1-0-ce57f301c7fd@linaro.org
---
Abel Vesa (7):
dt-bindings: spmi: Add X1E80100 SPMI PMIC ARB schema
dt-bindings: spmi: Deprecate qcom,bus-id
spmi: pmic-arb: Fix some compile warnings about members not being described
spmi: pmic-arb: Make the APID init a version operation
spmi: pmic-arb: Make core resources acquiring a version operation
spmi: pmic-arb: Register controller for bus instead of arbiter
spmi: pmic-arb: Add multi bus support
.../bindings/spmi/qcom,spmi-pmic-arb.yaml | 1 +
.../bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml | 136 +++
drivers/spmi/spmi-pmic-arb.c | 952 +++++++++++++--------
3 files changed, 723 insertions(+), 366 deletions(-)
---
base-commit: c0b832517f627ead3388c6f0c74e8ac10ad5774b
change-id: 20240207-spmi-multi-master-support-832a704b779b
Best regards,
--
Abel Vesa <abel.vesa@linaro.org>
^ permalink raw reply
* [PATCH v8 1/7] dt-bindings: spmi: Add X1E80100 SPMI PMIC ARB schema
From: Abel Vesa @ 2024-04-02 12:07 UTC (permalink / raw)
To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Abel Vesa,
Krzysztof Kozlowski
In-Reply-To: <20240402-spmi-multi-master-support-v8-0-ce6f2d14a058@linaro.org>
Add dedicated schema for X1E80100 PMIC ARB (v7) as it allows multiple
buses by declaring them as child nodes.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
.../bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml | 136 +++++++++++++++++++++
1 file changed, 136 insertions(+)
diff --git a/Documentation/devicetree/bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml
new file mode 100644
index 000000000000..f32a7ae33b4b
--- /dev/null
+++ b/Documentation/devicetree/bindings/spmi/qcom,x1e80100-spmi-pmic-arb.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spmi/qcom,x1e80100-spmi-pmic-arb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm X1E80100 SPMI Controller (PMIC Arbiter v7)
+
+maintainers:
+ - Stephen Boyd <sboyd@kernel.org>
+
+description: |
+ The X1E80100 SPMI PMIC Arbiter implements HW version 7 and it's an SPMI
+ controller with wrapping arbitration logic to allow for multiple on-chip
+ devices to control up to 2 SPMI separate buses.
+
+ The PMIC Arbiter can also act as an interrupt controller, providing interrupts
+ to slave devices.
+
+properties:
+ compatible:
+ const: qcom,x1e80100-spmi-pmic-arb
+
+ reg:
+ items:
+ - description: core registers
+ - description: tx-channel per virtual slave regosters
+ - description: rx-channel (called observer) per virtual slave registers
+
+ reg-names:
+ items:
+ - const: core
+ - const: chnls
+ - const: obsrvr
+
+ ranges: true
+
+ '#address-cells':
+ const: 2
+
+ '#size-cells':
+ const: 2
+
+ qcom,ee:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 5
+ description: >
+ indicates the active Execution Environment identifier
+
+ qcom,channel:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 5
+ description: >
+ which of the PMIC Arb provided channels to use for accesses
+
+patternProperties:
+ "^spmi@[a-f0-9]+$":
+ type: object
+ $ref: /schemas/spmi/spmi.yaml
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ items:
+ - description: configuration registers
+ - description: interrupt controller registers
+
+ reg-names:
+ items:
+ - const: cnfg
+ - const: intr
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ const: periph_irq
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 4
+ description: |
+ cell 1: slave ID for the requested interrupt (0-15)
+ cell 2: peripheral ID for requested interrupt (0-255)
+ cell 3: the requested peripheral interrupt (0-7)
+ cell 4: interrupt flags indicating level-sense information,
+ as defined in dt-bindings/interrupt-controller/irq.h
+
+required:
+ - compatible
+ - reg-names
+ - qcom,ee
+ - qcom,channel
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ spmi: arbiter@c400000 {
+ compatible = "qcom,x1e80100-spmi-pmic-arb";
+ reg = <0 0x0c400000 0 0x3000>,
+ <0 0x0c500000 0 0x4000000>,
+ <0 0x0c440000 0 0x80000>;
+ reg-names = "core", "chnls", "obsrvr";
+
+ qcom,ee = <0>;
+ qcom,channel = <0>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ spmi_bus0: spmi@c42d000 {
+ reg = <0 0x0c42d000 0 0x4000>,
+ <0 0x0c4c0000 0 0x10000>;
+ reg-names = "cnfg", "intr";
+
+ interrupt-names = "periph_irq";
+ interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-controller;
+ #interrupt-cells = <4>;
+
+ #address-cells = <2>;
+ #size-cells = <0>;
+ };
+ };
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH v8 2/7] dt-bindings: spmi: Deprecate qcom,bus-id
From: Abel Vesa @ 2024-04-02 12:07 UTC (permalink / raw)
To: Stephen Boyd, Matthias Brugger, Bjorn Andersson, Konrad Dybcio,
Dmitry Baryshkov, Neil Armstrong, AngeloGioacchino Del Regno,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Abel Vesa,
Krzysztof Kozlowski
In-Reply-To: <20240402-spmi-multi-master-support-v8-0-ce6f2d14a058@linaro.org>
As it is optional and no platform is actually using the secondary bus,
deprecate the qcom,bus-id property. For newer platforms that implement
SPMI PMIC ARB v7 in HW, the X1E80100 approach should be used.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
index f983b4af6db9..51daf1b847a9 100644
--- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
@@ -92,6 +92,7 @@ properties:
description: >
SPMI bus instance. only applicable to PMIC arbiter version 7 and beyond.
Supported values, 0 = primary bus, 1 = secondary bus
+ deprecated: true
required:
- compatible
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox