* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Boris Brezillon @ 2018-05-24 12:41 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
thierry.reding, mturquette, sboyd, dev, miquel.raynal, richard,
marcel, krzk, digetx, benjamin.lindqvist, jonathanh, pdeschrijver,
pgaikwad, mirza.krak, linux-mtd, linux-tegra, devicetree,
linux-kernel, linux-clk
In-Reply-To: <20180524142356.0fc68797@bbrezillon>
On Thu, 24 May 2018 14:23:56 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> On Thu, 24 May 2018 13:09:53 +0200
> Stefan Agner <stefan@agner.ch> wrote:
>
> > On 24.05.2018 10:56, Boris Brezillon wrote:
> > > On Thu, 24 May 2018 10:46:27 +0200
> > > Stefan Agner <stefan@agner.ch> wrote:
> > >
> > >> Hi Boris,
> > >>
> > >> Thanks for the initial review! One small question below:
> > >>
> > >> On 23.05.2018 16:18, Boris Brezillon wrote:
> > >> > Hi Stefan,
> > >> >
> > >> > On Tue, 22 May 2018 14:07:06 +0200
> > >> > Stefan Agner <stefan@agner.ch> wrote:
> > >> >> +
> > >> >> +struct tegra_nand {
> > >> >> + void __iomem *regs;
> > >> >> + struct clk *clk;
> > >> >> + struct gpio_desc *wp_gpio;
> > >> >> +
> > >> >> + struct nand_chip chip;
> > >> >> + struct device *dev;
> > >> >> +
> > >> >> + struct completion command_complete;
> > >> >> + struct completion dma_complete;
> > >> >> + bool last_read_error;
> > >> >> +
> > >> >> + dma_addr_t data_dma;
> > >> >> + void *data_buf;
> > >> >> + dma_addr_t oob_dma;
> > >> >> + void *oob_buf;
> > >> >> +
> > >> >> + int cur_chip;
> > >> >> +};
> > >> >
> > >> > This struct should be split in 2 structures: one representing the NAND
> > >> > controller and one representing the NAND chip:
> > >> >
> > >> > struct tegra_nand_controller {
> > >> > struct nand_hw_control base;
> > >> > void __iomem *regs;
> > >> > struct clk *clk;
> > >> > struct device *dev;
> > >> > struct completion command_complete;
> > >> > struct completion dma_complete;
> > >> > bool last_read_error;
> > >> > int cur_chip;
> > >> > };
> > >> >
> > >> > struct tegra_nand {
> > >> > struct nand_chip base;
> > >> > dma_addr_t data_dma;
> > >> > void *data_buf;
> > >> > dma_addr_t oob_dma;
> > >> > void *oob_buf;
> > >> > };
> > >>
> > >> Is there a particular reason why you would leave DMA buffers in the chip
> > >> structure? It seems that is more a controller thing...
> > >
> > > The size of those buffers is likely to be device dependent, so if you
> > > have several NANDs connected to the controller, you'll either have to
> > > have one buffer at the controller level which is max(all-chip-buf-size)
> > > or a buffer per device.
> > >
> > > Also, do you really need these buffers? The core already provide some
> > > which are suitable for DMA (chip->oob_poi and chip->data_buf).
> > >
> >
> > Good question, I am not sure, that was existing code.
> >
> > Are you sure data_buf it is DMA capable?
> >
> > nand_scan_tail allocates with kmalloc:
> >
> > chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
>
> Yes, kmalloc() allocates DMA-able buffers, so those are DMA-safe.
Hm, that's not exactly true. It depends on the dma_mask attached to the
device.
^ permalink raw reply
* Re: [PATCH V1 2/3] mmc: sdhci-msm: Add msm version specific ops and data structures
From: Vijay Viswanath @ 2018-05-24 12:35 UTC (permalink / raw)
To: Evan Green
Cc: adrian.hunter, Ulf Hansson, robh+dt, mark.rutland, linux-mmc,
linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, devicetree,
asutoshd, stummala, venkatg, jeremymc, Bjorn Andersson, riteshh,
vbadigan, Doug Anderson, sayalil
In-Reply-To: <CAE=gft4ZQ34QankD_S5y_KjoZG8rxQkEfPo7vZsJTUB387P6dQ@mail.gmail.com>
On 5/22/2018 11:40 PM, Evan Green wrote:
> On Thu, May 17, 2018 at 3:30 AM Vijay Viswanath <vviswana@codeaurora.org>
> wrote:
>
>> In addition to offsets of certain registers changing, the registers in
>> core_mem have been shifted to HC mem as well. To access these registers,
>> define msm version specific functions. These functions can be loaded
>> into the function pointers at the time of probe based on the msm version
>> detected.
>
>> Also defind new data structure to hold version specific Ops and register
>> addresses.
>
>> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
>> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 112
> +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 112 insertions(+)
>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 2524455..bb2bb59 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -226,6 +226,25 @@ struct sdhci_msm_offset {
>> .core_ddr_config_2 = 0x1BC,
>> };
>
>> +struct sdhci_msm_variant_ops {
>> + u8 (*msm_readb_relaxed)(struct sdhci_host *host, u32 offset);
>
> I don't see any uses of msm_readb_relaxed or msm_writeb_relaxed in this
> patch or the next one. Are these needed?
They are not used as of now. Kept them since they can have use later.
Felt it better to define base functions and addresses now itself.
>
>> + u32 (*msm_readl_relaxed)(struct sdhci_host *host, u32 offset);
>> + void (*msm_writeb_relaxed)(u8 val, struct sdhci_host *host, u32
> offset);
>> + void (*msm_writel_relaxed)(u32 val, struct sdhci_host *host,
>> + u32 offset);
>> +};
>> +
>> +/*
>> + * From V5, register spaces have changed. Wrap this info in a structure
>> + * and choose the data_structure based on version info mentioned in DT.
>> + */
>> +struct sdhci_msm_variant_info {
>> + bool mci_removed;
>> + const struct sdhci_msm_variant_ops *var_ops;
>> + const struct sdhci_msm_offset *offset;
>> +};
>> +
>> +
>
> Remove extra blank line.
>
>> struct sdhci_msm_host {
>> struct platform_device *pdev;
>> void __iomem *core_mem; /* MSM SDCC mapped address */
>> @@ -245,8 +264,75 @@ struct sdhci_msm_host {
>> wait_queue_head_t pwr_irq_wait;
>> bool pwr_irq_flag;
>> u32 caps_0;
>> + bool mci_removed;
>> + const struct sdhci_msm_variant_ops *var_ops;
>> + const struct sdhci_msm_offset *offset;
>> };
>
>> +/*
>> + * APIs to read/write to vendor specific registers which were there in
> the
>> + * core_mem region before MCI was removed.
>> + */
>> +static u8 sdhci_msm_mci_variant_readb_relaxed(struct sdhci_host *host,
>> + u32 offset)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> + return readb_relaxed(msm_host->core_mem + offset);
>> +}
>> +
>> +static u8 sdhci_msm_v5_variant_readb_relaxed(struct sdhci_host *host,
>> + u32 offset)
>> +{
>> + return readb_relaxed(host->ioaddr + offset);
>> +}
>> +
>> +static u32 sdhci_msm_mci_variant_readl_relaxed(struct sdhci_host *host,
>> + u32 offset)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> + return readl_relaxed(msm_host->core_mem + offset);
>> +}
>> +
>> +static u32 sdhci_msm_v5_variant_readl_relaxed(struct sdhci_host *host,
>> + u32 offset)
>> +{
>> + return readl_relaxed(host->ioaddr + offset);
>> +}
>> +
>> +static void sdhci_msm_mci_variant_writeb_relaxed(u8 val,
>> + struct sdhci_host *host, u32 offset)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> + writeb_relaxed(val, msm_host->core_mem + offset);
>> +}
>> +
>> +static void sdhci_msm_v5_variant_writeb_relaxed(u8 val, struct
> sdhci_host *host,
>> + u32 offset)
>> +{
>> + writeb_relaxed(val, host->ioaddr + offset);
>> +}
>> +
>> +static void sdhci_msm_mci_variant_writel_relaxed(u32 val,
>> + struct sdhci_host *host, u32 offset)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> + writel_relaxed(val, msm_host->core_mem + offset);
>> +}
>> +
>> +static void sdhci_msm_v5_variant_writel_relaxed(u32 val, struct
> sdhci_host *host,
>
> You squeaked over 80 characters here. Move the second parameter down with
> the third.
>
> -Evan
>
Thanks for going through the patch thoroughly. Will address the comments.
Thanks,
Vijay
^ permalink raw reply
* Re: [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Xu Zaibo @ 2018-05-24 12:35 UTC (permalink / raw)
To: Jean-Philippe Brucker,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, liguozhu,
christian.koenig-5C7GfCeVMHo
In-Reply-To: <cd13f60d-b282-3804-4ca7-2d34476c597f-5wv7dgnIgG8@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2914 bytes --]
On 2018/5/24 19:44, Jean-Philippe Brucker wrote:
> Hi,
>
> On 23/05/18 10:38, Xu Zaibo wrote:
>>> +static int vfio_iommu_bind_group(struct vfio_iommu *iommu,
>>> + struct vfio_group *group,
>>> + struct vfio_mm *vfio_mm)
>>> +{
>>> + int ret;
>>> + bool enabled_sva = false;
>>> + struct vfio_iommu_sva_bind_data data = {
>>> + .vfio_mm = vfio_mm,
>>> + .iommu = iommu,
>>> + .count = 0,
>>> + };
>>> +
>>> + if (!group->sva_enabled) {
>>> + ret = iommu_group_for_each_dev(group->iommu_group, NULL,
>>> + vfio_iommu_sva_init);
>> Do we need to do *sva_init here or do anything to avoid repeated
>> initiation?
>> while another process already did initiation at this device, I think
>> that current process will get an EEXIST.
> Right, sva_init() must be called once for any device that intends to use
> bind(). For the second process though, group->sva_enabled will be true
> so we won't call sva_init() again, only bind().
>
Well, while I create mediated devices based on one parent device to
support multiple
processes(a new process will create a new 'vfio_group' for the
corresponding mediated device,
and 'sva_enabled' cannot work any more), in fact, *sva_init and
*sva_shutdown are basically
working on parent device, so, as a result, I just only need sva
initiation and shutdown on the
parent device only once. So I change the two as following:
/@@ -551,8 +565,18 @@ int iommu_sva_device_init(struct device *dev,
unsigned long features,/
if (features & ~IOMMU_SVA_FEAT_IOPF)
return -EINVAL;
/+ /* If already exists, do nothing *///
//+ mutex_lock(&dev->iommu_param->lock);//
//+ if (dev->iommu_param->sva_param) {//
//+ mutex_unlock(&dev->iommu_param->lock);//
//+ return 0;//
//+ }//
//+ mutex_unlock(&dev->iommu_param->lock);//
////
// if (features & IOMMU_SVA_FEAT_IOPF) {//
// ret = iommu_register_device_fault_handler(dev,
iommu_queue_iopf,///// /
//
//
//@@ -621,6 +646,14 @@ int iommu_sva_device_shutdown(struct device *dev)//
// if (!domain)//
// return -ENODEV;//
////
//+ /* If any other process is working on the device, shut down does
nothing. *///
//+ mutex_lock(&dev->iommu_param->lock);//
//+ if (!list_empty(&dev->iommu_param->sva_param->mm_list)) {//
//+ mutex_unlock(&dev->iommu_param->lock);//
//+ return 0;//
//+ }//
//+ mutex_unlock(&dev->iommu_param->lock);//
//+//
// __iommu_sva_unbind_dev_all(dev);//
////
// mutex_lock(&dev->iommu_param->lock);/
I add the above two checkings in both *sva_init and *sva_shutdown, it is
working now,
but i don't know if it will cause any new problems. What's more, i doubt
if it is
reasonable to check this to avoid repeating operation in VFIO, maybe it
is better to check
in IOMMU. :)
Thanks
Zaibo
>
> .
>
[-- Attachment #1.2: Type: text/html, Size: 42100 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Boris Brezillon @ 2018-05-24 12:27 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
thierry.reding, mturquette, sboyd, dev, miquel.raynal, richard,
marcel, krzk, digetx, benjamin.lindqvist, jonathanh, pdeschrijver,
pgaikwad, mirza.krak, linux-mtd, linux-tegra, devicetree,
linux-kernel, linux-clk
In-Reply-To: <7b3cc3991fb054130fd54c6fdfec5097@agner.ch>
On Thu, 24 May 2018 13:09:53 +0200
Stefan Agner <stefan@agner.ch> wrote:
> On 24.05.2018 10:56, Boris Brezillon wrote:
> > On Thu, 24 May 2018 10:46:27 +0200
> > Stefan Agner <stefan@agner.ch> wrote:
> >
> >> Hi Boris,
> >>
> >> Thanks for the initial review! One small question below:
> >>
> >> On 23.05.2018 16:18, Boris Brezillon wrote:
> >> > Hi Stefan,
> >> >
> >> > On Tue, 22 May 2018 14:07:06 +0200
> >> > Stefan Agner <stefan@agner.ch> wrote:
> >> >> +
> >> >> +struct tegra_nand {
> >> >> + void __iomem *regs;
> >> >> + struct clk *clk;
> >> >> + struct gpio_desc *wp_gpio;
> >> >> +
> >> >> + struct nand_chip chip;
> >> >> + struct device *dev;
> >> >> +
> >> >> + struct completion command_complete;
> >> >> + struct completion dma_complete;
> >> >> + bool last_read_error;
> >> >> +
> >> >> + dma_addr_t data_dma;
> >> >> + void *data_buf;
> >> >> + dma_addr_t oob_dma;
> >> >> + void *oob_buf;
> >> >> +
> >> >> + int cur_chip;
> >> >> +};
> >> >
> >> > This struct should be split in 2 structures: one representing the NAND
> >> > controller and one representing the NAND chip:
> >> >
> >> > struct tegra_nand_controller {
> >> > struct nand_hw_control base;
> >> > void __iomem *regs;
> >> > struct clk *clk;
> >> > struct device *dev;
> >> > struct completion command_complete;
> >> > struct completion dma_complete;
> >> > bool last_read_error;
> >> > int cur_chip;
> >> > };
> >> >
> >> > struct tegra_nand {
> >> > struct nand_chip base;
> >> > dma_addr_t data_dma;
> >> > void *data_buf;
> >> > dma_addr_t oob_dma;
> >> > void *oob_buf;
> >> > };
> >>
> >> Is there a particular reason why you would leave DMA buffers in the chip
> >> structure? It seems that is more a controller thing...
> >
> > The size of those buffers is likely to be device dependent, so if you
> > have several NANDs connected to the controller, you'll either have to
> > have one buffer at the controller level which is max(all-chip-buf-size)
> > or a buffer per device.
> >
> > Also, do you really need these buffers? The core already provide some
> > which are suitable for DMA (chip->oob_poi and chip->data_buf).
> >
>
> Good question, I am not sure, that was existing code.
>
> Are you sure data_buf it is DMA capable?
>
> nand_scan_tail allocates with kmalloc:
>
> chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
Also, you might want to set the NAND_USE_BOUNCE_BUFFER flag in
chip->options, so that the core always pass DMA-able buffers to your
->read/write_page[_raw] function.
[1]https://elixir.bootlin.com/linux/v4.17-rc6/source/include/linux/mtd/rawnand.h#L202
^ permalink raw reply
* Re: [PATCH V1 1/3] mmc: sdhci-msm: Define new Register address map
From: Vijay Viswanath @ 2018-05-24 12:27 UTC (permalink / raw)
To: Evan Green
Cc: adrian.hunter, Ulf Hansson, robh+dt, mark.rutland, linux-mmc,
linux-kernel, shawn.lin, linux-arm-msm, georgi.djakov, devicetree,
asutoshd, stummala, venkatg, jeremymc, Bjorn Andersson, riteshh,
vbadigan, Doug Anderson, sayalil
In-Reply-To: <CAE=gft7uoUSDmfrpzEQ_AGKPD3E25X6xwPiTkgsJwZn-zHaYuQ@mail.gmail.com>
On 5/22/2018 11:39 PM, Evan Green wrote:
> Hi Vijay,
> On Thu, May 17, 2018 at 3:30 AM Vijay Viswanath <vviswana@codeaurora.org>
> wrote:
>
>> From: Sayali Lokhande <sayalil@codeaurora.org>
>
>> For SDCC version 5.0.0, MCI registers are removed from SDCC
>> interface and some registers are moved to HC.
>> Define a new data structure where we can statically define
>> the address offsets for the registers in different SDCC versions.
>
>> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
>> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 89
> ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index bb11916..2524455 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -137,6 +137,95 @@
>> /* Timeout value to avoid infinite waiting for pwr_irq */
>> #define MSM_PWR_IRQ_TIMEOUT_MS 5000
>
>> +struct sdhci_msm_offset {
>> + u32 core_hc_mode;
>> + u32 core_mci_data_cnt;
>> + u32 core_mci_status;
>> + u32 core_mci_fifo_cnt;
>> + u32 core_mci_version;
>> + u32 core_generics;
>> + u32 core_testbus_config;
>> + u32 core_testbus_sel2_bit;
>> + u32 core_testbus_ena;
>> + u32 core_testbus_sel2;
>> + u32 core_pwrctl_status;
>> + u32 core_pwrctl_mask;
>> + u32 core_pwrctl_clear;
>> + u32 core_pwrctl_ctl;
>> + u32 core_sdcc_debug_reg;
>> + u32 core_dll_config;
>> + u32 core_dll_status;
>> + u32 core_vendor_spec;
>> + u32 core_vendor_spec_adma_err_addr0;
>> + u32 core_vendor_spec_adma_err_addr1;
>> + u32 core_vendor_spec_func2;
>> + u32 core_vendor_spec_capabilities0;
>> + u32 core_ddr_200_cfg;
>> + u32 core_vendor_spec3;
>> + u32 core_dll_config_2;
>> + u32 core_ddr_config;
>> + u32 core_ddr_config_2;
>> +};
>> +
>> +static const struct sdhci_msm_offset sdhci_msm_v5_offset = {
>> + .core_mci_data_cnt = 0x35c,
>> + .core_mci_status = 0x324,
>> + .core_mci_fifo_cnt = 0x308,
>> + .core_mci_version = 0x318,
>> + .core_generics = 0x320,
>> + .core_testbus_config = 0x32c,
>> + .core_testbus_sel2_bit = 3,
>> + .core_testbus_ena = (1 << 31),
>> + .core_testbus_sel2 = (1 << 3),
>> + .core_pwrctl_status = 0x240,
>> + .core_pwrctl_mask = 0x244,
>> + .core_pwrctl_clear = 0x248,
>> + .core_pwrctl_ctl = 0x24c,
>> + .core_sdcc_debug_reg = 0x358,
>> + .core_dll_config = 0x200,
>> + .core_dll_status = 0x208,
>> + .core_vendor_spec = 0x20c,
>> + .core_vendor_spec_adma_err_addr0 = 0x214,
>> + .core_vendor_spec_adma_err_addr1 = 0x218,
>> + .core_vendor_spec_func2 = 0x210,
>> + .core_vendor_spec_capabilities0 = 0x21c,
>> + .core_ddr_200_cfg = 0x224,
>> + .core_vendor_spec3 = 0x250,
>> + .core_dll_config_2 = 0x254,
>> + .core_ddr_config = 0x258,
>> + .core_ddr_config_2 = 0x25c,
>> +};
>> +
>> +static const struct sdhci_msm_offset sdhci_msm_mci_offset = {
>> + .core_hc_mode = 0x78,
>> + .core_mci_data_cnt = 0x30,
>> + .core_mci_status = 0x34,
>> + .core_mci_fifo_cnt = 0x44,
>> + .core_mci_version = 0x050,
>> + .core_generics = 0x70,
>> + .core_testbus_config = 0x0CC,
>> + .core_testbus_sel2_bit = 4,
>> + .core_testbus_ena = (1 << 3),
>> + .core_testbus_sel2 = (1 << 4),
>> + .core_pwrctl_status = 0xDC,
>> + .core_pwrctl_mask = 0xE0,
>> + .core_pwrctl_clear = 0xE4,
>> + .core_pwrctl_ctl = 0xE8,
>> + .core_sdcc_debug_reg = 0x124,
>> + .core_dll_config = 0x100,
>> + .core_dll_status = 0x108,
>> + .core_vendor_spec = 0x10C,
>> + .core_vendor_spec_adma_err_addr0 = 0x114,
>> + .core_vendor_spec_adma_err_addr1 = 0x118,
>> + .core_vendor_spec_func2 = 0x110,
>> + .core_vendor_spec_capabilities0 = 0x11C,
>> + .core_ddr_200_cfg = 0x184,
>> + .core_vendor_spec3 = 0x1B0,
>> + .core_dll_config_2 = 0x1B4,
>> + .core_ddr_config = 0x1B8,
>> + .core_ddr_config_2 = 0x1BC,
>> +};
>> +
>
> I notice a lot of these are never used in the subsequent patches of this
> series. I guess more register definitions are always better than fewer,
> it's just a shame that they take up space now. Did you just add everything
> that was different between v4 and v5, or how did you come up with this set?
>
> Also, I think lowercase hex letters are preferred.
>
> I verified that the v5 register offsets look good, at least for the
> registers I have documentation for.
>
Yeah, felt it better to include all registers even they are not used
currently.
Will change to use lowercase hex letters
> -Evan
>
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Boris Brezillon @ 2018-05-24 12:23 UTC (permalink / raw)
To: Stefan Agner
Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
thierry.reding, mturquette, sboyd, dev, miquel.raynal, richard,
marcel, krzk, digetx, benjamin.lindqvist, jonathanh, pdeschrijver,
pgaikwad, mirza.krak, linux-mtd, linux-tegra, devicetree,
linux-kernel, linux-clk
In-Reply-To: <7b3cc3991fb054130fd54c6fdfec5097@agner.ch>
On Thu, 24 May 2018 13:09:53 +0200
Stefan Agner <stefan@agner.ch> wrote:
> On 24.05.2018 10:56, Boris Brezillon wrote:
> > On Thu, 24 May 2018 10:46:27 +0200
> > Stefan Agner <stefan@agner.ch> wrote:
> >
> >> Hi Boris,
> >>
> >> Thanks for the initial review! One small question below:
> >>
> >> On 23.05.2018 16:18, Boris Brezillon wrote:
> >> > Hi Stefan,
> >> >
> >> > On Tue, 22 May 2018 14:07:06 +0200
> >> > Stefan Agner <stefan@agner.ch> wrote:
> >> >> +
> >> >> +struct tegra_nand {
> >> >> + void __iomem *regs;
> >> >> + struct clk *clk;
> >> >> + struct gpio_desc *wp_gpio;
> >> >> +
> >> >> + struct nand_chip chip;
> >> >> + struct device *dev;
> >> >> +
> >> >> + struct completion command_complete;
> >> >> + struct completion dma_complete;
> >> >> + bool last_read_error;
> >> >> +
> >> >> + dma_addr_t data_dma;
> >> >> + void *data_buf;
> >> >> + dma_addr_t oob_dma;
> >> >> + void *oob_buf;
> >> >> +
> >> >> + int cur_chip;
> >> >> +};
> >> >
> >> > This struct should be split in 2 structures: one representing the NAND
> >> > controller and one representing the NAND chip:
> >> >
> >> > struct tegra_nand_controller {
> >> > struct nand_hw_control base;
> >> > void __iomem *regs;
> >> > struct clk *clk;
> >> > struct device *dev;
> >> > struct completion command_complete;
> >> > struct completion dma_complete;
> >> > bool last_read_error;
> >> > int cur_chip;
> >> > };
> >> >
> >> > struct tegra_nand {
> >> > struct nand_chip base;
> >> > dma_addr_t data_dma;
> >> > void *data_buf;
> >> > dma_addr_t oob_dma;
> >> > void *oob_buf;
> >> > };
> >>
> >> Is there a particular reason why you would leave DMA buffers in the chip
> >> structure? It seems that is more a controller thing...
> >
> > The size of those buffers is likely to be device dependent, so if you
> > have several NANDs connected to the controller, you'll either have to
> > have one buffer at the controller level which is max(all-chip-buf-size)
> > or a buffer per device.
> >
> > Also, do you really need these buffers? The core already provide some
> > which are suitable for DMA (chip->oob_poi and chip->data_buf).
> >
>
> Good question, I am not sure, that was existing code.
>
> Are you sure data_buf it is DMA capable?
>
> nand_scan_tail allocates with kmalloc:
>
> chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
Yes, kmalloc() allocates DMA-able buffers, so those are DMA-safe.
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-05-24 12:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Benjamin Lindqvist, dwmw2, computersforpeace, marek.vasut,
robh+dt, mark.rutland, thierry.reding, mturquette, sboyd,
Lucas Stach, miquel.raynal, richard, marcel, krzk, digetx,
jonathanh, pdeschrijver, pgaikwad, Mirza Krak, linux-mtd,
linux-tegra, devicetree, linux-kernel, linux-clk
In-Reply-To: <20180524135335.6aa0b7a4@bbrezillon>
On 24.05.2018 13:53, Boris Brezillon wrote:
> Hi Benjamin,
>
> On Thu, 24 May 2018 13:30:14 +0200
> Benjamin Lindqvist <benjamin.lindqvist@endian.se> wrote:
>
>> Hi Stefan,
>>
>> It seems to me that a probe similar to what the BootROM does shouldn't
>> be awfully complicated to implement - just cycle through the switch
>> cases in case of an ECC error. But I guess that's more of an idea for
>> further improvements rather than a comment to the patch set under
>> review.
>
> Nope, not really an option, because you're not guaranteed that the NAND
> will be used as a boot media, and the first page or first set of pages
> might just be erased.
>
Yeah I did not meant probing like the Boot ROM does.
What I meant was using only the ECC modes which are supported by the
Boot ROM when the driver tries to choose a viable mode. So that would
be:
- RS t=4
- BCH t=8
- BCH t=16
Maybe we could add a property to enable that behavior:
tegra,use-bootable-ecc-only;
>>
>> However, I think that allowing for an override of the oobsize
>> inference would be a good idea before merging, no? This could just be
>> a trivial #ifdef (at least temporarily). If you agree but don't feel
>> like doing it yourself, I'd be happy to pitch in. Let me know.
>
> That's why we have nand-ecc-xxx properties in the DT.
>
Yes, nand-ecc-strength is the first thing I plan to implement, that way
strength can be defined in dt.
--
Stefan
^ permalink raw reply
* Re: [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Heiko Stuebner @ 2018-05-24 12:18 UTC (permalink / raw)
To: Levin Du
Cc: Rob Herring, open list:ARM/Rockchip SoC..., Wayne Chou,
devicetree, Linus Walleij, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM, Mark Rutland,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <860b225a-94e0-c3cb-94c2-5e354e0ccb1f@t-chip.com.cn>
Hi Levin,
Am Donnerstag, 24. Mai 2018, 03:59:36 CEST schrieb Levin Du:
> Hi all, I'd like to quote reply of Robin Murphy at
> http://lists.infradead.org/pipermail/linux-rockchip/2018-May/020619.html
>
> >
> > I would suggest s/pin number/bit number in the associated GRF register/
> > here. At least in this RK3328 case there's only one pin, which isn't
> > numbered, and if you naively considered it pin 0 of this 'bank' you'd
> > already have the wrong number. Since we're dealing with the "random
> > SoC-specific controls" region of the GRF as opposed to the
> > relatively-consistent and organised pinmux parts, I don't think we
> > should rely on any assumptions about how things are laid out.
> >
> > I was initially going to suggest a more specific compatible string as
> > well, but on reflection I think the generic "rockchip,gpio-syscon" for
> > basic "flip this single GRF bit" functionality actually is the right way
> > to go. In the specific RK3328 GPIO_MUTE case, there look to be 4 bits in
> > total related to this pin - the enable, value, and some pull controls
> > (which I assume apply when the output is disabled) - if at some point in
> > future we *did* want to start explicitly controlling the rest of them
> > too, then would be a good time to define a separate
> > "rockchip,rk3328-gpio-mute" binding (and probably a dedicated driver)
> > for that specialised functionality, independently of this basic one.
>
>
> Shall we go the generic "rockchip,gpio-syscon" way, or the specific
> "rockchip,rk3328-gpio-mute" way? I prefer the former one.
>
> The property of "gpio,syscon-dev" in gpio-syscon driver should be
> documented.
> Since the gpio controller is defined in the dtsi file, which inevitably
> contains voodoo
> register addresses. But at the board level dts file, there won't be more
> register
> addresses.
Past experience shows that the GRF is not really suitable for
generalization, as it's more of a dumping ground where chip designers
can put everything that's left over. This is especially true for
GRF_SOC_CONx registers, that really only contain pretty random bits.
So personally, I'd really prefer soc-specific compatibles as everywhere
else, instead of trying to push stuff into the devicetree that won't hold
up on future socs.
> On 2018-05-24 3:53 AM, Rob Herring wrote:
> > On Wed, May 23, 2018 at 10:12 AM, Heiko Stübner <heiko@sntech.de> wrote:
> >> Hi Rob, Levin,
> >>
> >> sorry for being late to the party.
> >>
> >> Am Mittwoch, 23. Mai 2018, 16:43:07 CEST schrieb Rob Herring:
> >>> On Tue, May 22, 2018 at 9:02 PM, Levin Du <djw@t-chip.com.cn> wrote:
> >>>> On 2018-05-23 2:02 AM, Rob Herring wrote:
> >>>>> On Fri, May 18, 2018 at 11:52:05AM +0800, djw@t-chip.com.cn wrote:
> >>>>>> From: Levin Du <djw@t-chip.com.cn>
> >>>>>>
> >>>>>> Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs,
> >>>>>> which do not belong to the general pinctrl.
> >>>>>>
> >>>>>> Adding gpio-syscon support makes controlling regulator or
> >>>>>> LED using these special pins very easy by reusing existing
> >>>>>> drivers, such as gpio-regulator and led-gpio.
> >>>>>>
> >>>>>> Signed-off-by: Levin Du <djw@t-chip.com.cn>
> >>>>>>
> >>>>>> ---
> >>>>>>
> >>>>>> Changes in v2:
> >>>>>> - Rename gpio_syscon10 to gpio_mute in doc
> >>>>>>
> >>>>>> Changes in v1:
> >>>>>> - Refactured for general gpio-syscon usage for Rockchip SoCs.
> >>>>>> - Add doc rockchip,gpio-syscon.txt
> >>>>>>
> >>>>>> .../bindings/gpio/rockchip,gpio-syscon.txt | 41
> >>>>>>
> >>>>>> ++++++++++++++++++++++
> >>>>>>
> >>>>>> drivers/gpio/gpio-syscon.c | 30
> >>>>>>
> >>>>>> ++++++++++++++++
> >>>>>>
> >>>>>> 2 files changed, 71 insertions(+)
> >>>>>> create mode 100644
> >>>>>>
> >>>>>> Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >>>>>>
> >>>>>> diff --git
> >>>>>> a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >>>>>> b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >>>>>> new file mode 100644
> >>>>>> index 0000000..b1b2a67
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >>>>>> @@ -0,0 +1,41 @@
> >>>>>> +* Rockchip GPIO support for GRF_SOC_CON registers
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +- compatible: Should contain "rockchip,gpio-syscon".
> >>>>>> +- gpio-controller: Marks the device node as a gpio controller.
> >>>>>> +- #gpio-cells: Should be two. The first cell is the pin number and
> >>>>>> + the second cell is used to specify the gpio polarity:
> >>>>>> + 0 = Active high,
> >>>>>> + 1 = Active low.
> >>>>> There's no need for this child node. Just make the parent node a gpio
> >>>>> controller.
> >>>>>
> >>>>> Rob
> >>>> Hi Rob, it is not clear to me. Do you suggest that the grf node should be
> >>>> a
> >>>> gpio controller,
> >>>> like below?
> >>>>
> >>>> + grf: syscon at ff100000 {
> >>>> + compatible = "rockchip,gpio-syscon", "rockchip,rk3328-grf",
> >>>> "syscon", "simple-mfd";
> >>> Yes, but drop "rockchip,gpio-syscon" and "simple-mfd".
> >> I would disagree quite a bit here. The grf are the "general register files",
> >> a bunch of registers used for quite a lot of things, and so it seems
> >> among other users, also a gpio-controller for some more random pins
> >> not controlled through the regular gpio controllers.
> >>
> >> For a more fully stocked grf, please see
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/rk3288.dtsi#n855
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399.dtsi#n1338
> >>
> >> So the gpio controller should definitly also be a subnode.
> > Sigh, yes, if there are a bunch of functions needing subnodes like the
> > above, then yes that makes sense. But that's not what has been
> > presented. Please make some attempt at defining *all* the functions.
> > An actual binding would be nice, but I'll settle for just a list of
> > things. The list should have functions that have DT dependencies (like
> > clocks for phys in the above) because until you do, you don't need
> > child nodes.
>
> In rk3328.dtsi file, there are lots of line "rockchip,grf = <&grf>;" in
> various nodes,
> such as tsadc, cru, gmac2io, gmac2phy, and also pinctrl, which are not
> sub nodes of
> `grf`, but for reference only. The gpio-syscon node should also have
> similar behavior.
> They are not strongly coupled. The gpio-syscon node should be defined
> outside of the
> `grf` node.
Not necessarily.
I.e. things like the tsadc, cru etc have their own register space and only
some minor additional bits inside the GRF.
Other things like some phys and your mute-gpio are _fully embedded_ inside
the GRF and thus become child devices. This describes the hardware layout
way better, helps unclutter the devicetree and also shows this distinction
between "additional bits" and "embedded" clearly.
Heiko
^ permalink raw reply
* Re: [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Heiko Stuebner @ 2018-05-24 12:07 UTC (permalink / raw)
To: Rob Herring
Cc: Levin Du, open list:ARM/Rockchip SoC..., Wayne Chou, devicetree,
Linus Walleij, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM, Mark Rutland,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_Jsq+VBspzCaLD2Wy=fyC-S+LKGZbVsB+b+Etkx91ioXOCuQ@mail.gmail.com>
Hi Rob,
Am Mittwoch, 23. Mai 2018, 21:53:53 CEST schrieb Rob Herring:
> On Wed, May 23, 2018 at 10:12 AM, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Mittwoch, 23. Mai 2018, 16:43:07 CEST schrieb Rob Herring:
> >> On Tue, May 22, 2018 at 9:02 PM, Levin Du <djw@t-chip.com.cn> wrote:
> >> > On 2018-05-23 2:02 AM, Rob Herring wrote:
> >> >> On Fri, May 18, 2018 at 11:52:05AM +0800, djw@t-chip.com.cn wrote:
> >> >>> From: Levin Du <djw@t-chip.com.cn>
> >> >>>
> >> >>> Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs,
> >> >>> which do not belong to the general pinctrl.
> >> >>>
> >> >>> Adding gpio-syscon support makes controlling regulator or
> >> >>> LED using these special pins very easy by reusing existing
> >> >>> drivers, such as gpio-regulator and led-gpio.
> >> >>>
> >> >>> Signed-off-by: Levin Du <djw@t-chip.com.cn>
> >> >>>
> >> >>> ---
> >> >>>
> >> >>> Changes in v2:
> >> >>> - Rename gpio_syscon10 to gpio_mute in doc
> >> >>>
> >> >>> Changes in v1:
> >> >>> - Refactured for general gpio-syscon usage for Rockchip SoCs.
> >> >>> - Add doc rockchip,gpio-syscon.txt
> >> >>>
> >> >>> .../bindings/gpio/rockchip,gpio-syscon.txt | 41
> >> >>>
> >> >>> ++++++++++++++++++++++
> >> >>>
> >> >>> drivers/gpio/gpio-syscon.c | 30
> >> >>>
> >> >>> ++++++++++++++++
> >> >>>
> >> >>> 2 files changed, 71 insertions(+)
> >> >>> create mode 100644
> >> >>>
> >> >>> Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >> >>>
> >> >>> diff --git
> >> >>> a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >> >>> b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >> >>> new file mode 100644
> >> >>> index 0000000..b1b2a67
> >> >>> --- /dev/null
> >> >>> +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
> >> >>> @@ -0,0 +1,41 @@
> >> >>> +* Rockchip GPIO support for GRF_SOC_CON registers
> >> >>> +
> >> >>> +Required properties:
> >> >>> +- compatible: Should contain "rockchip,gpio-syscon".
> >> >>> +- gpio-controller: Marks the device node as a gpio controller.
> >> >>> +- #gpio-cells: Should be two. The first cell is the pin number and
> >> >>> + the second cell is used to specify the gpio polarity:
> >> >>> + 0 = Active high,
> >> >>> + 1 = Active low.
> >> >>
> >> >> There's no need for this child node. Just make the parent node a gpio
> >> >> controller.
> >> >>
> >> >> Rob
> >> >
> >> > Hi Rob, it is not clear to me. Do you suggest that the grf node should be
> >> > a
> >> > gpio controller,
> >> > like below?
> >> >
> >> > + grf: syscon at ff100000 {
> >> > + compatible = "rockchip,gpio-syscon", "rockchip,rk3328-grf",
> >> > "syscon", "simple-mfd";
> >>
> >> Yes, but drop "rockchip,gpio-syscon" and "simple-mfd".
> >
> > I would disagree quite a bit here. The grf are the "general register files",
> > a bunch of registers used for quite a lot of things, and so it seems
> > among other users, also a gpio-controller for some more random pins
> > not controlled through the regular gpio controllers.
> >
> > For a more fully stocked grf, please see
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/rk3288.dtsi#n855
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399.dtsi#n1338
> >
> > So the gpio controller should definitly also be a subnode.
>
> Sigh, yes, if there are a bunch of functions needing subnodes like the
> above, then yes that makes sense. But that's not what has been
> presented. Please make some attempt at defining *all* the functions.
> An actual binding would be nice, but I'll settle for just a list of
> things. The list should have functions that have DT dependencies (like
> clocks for phys in the above) because until you do, you don't need
> child nodes.
That's the problem with the Rockchip-GRF, you only realize its content
when implementing specific features.
Like on the rk3399 the table of the register-list of the GRF alone is 11
pages long with the register details tables taking up another 230 pages.
And functional description is often somewhat thin.
So I'm not sure I fully understand what you're asking, but in general
we define the bindings for sub-devices when tackling these individual
components, see for example
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=72580a49a837c2c7da83f698c00592eac41537d8
which also has a real phy-driver behind it and binding against that
subnode of the GRF simple-mfd.
These are real IP blocks somewhere on the socs, with regular supplies
like resets, clocks etc in most cases. Only their controlling registers
got dumped into the GRF for some reason.
And in retrospect it really looks like we're doing something right,
because it seems these bindings seem quite stable over time.
> > The gpio in question is called "mute", so I'd think the gpio-syscon driver
> > should just define a "rockchip,rk3328-gpio-mute" compatible and contain
> > all the register voodoo in the driver itself and not define it in the dt.
>
> Is there really just one GPIO? If it has a defined function, then is
> it really GP? Can you control direction? I know Linus W doesn't like
> that kind of abuse of GPIO.
looks like I convinced Linus that we're not abusing anything with this :-) .
Heiko
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Boris Brezillon @ 2018-05-24 11:53 UTC (permalink / raw)
To: Benjamin Lindqvist
Cc: Stefan Agner, dwmw2, computersforpeace, marek.vasut, robh+dt,
mark.rutland, thierry.reding, mturquette, sboyd, Lucas Stach,
miquel.raynal, richard, marcel, krzk, digetx, jonathanh,
pdeschrijver, pgaikwad, Mirza Krak, linux-mtd, linux-tegra,
devicetree, linux-kernel, linux-clk
In-Reply-To: <CAGefG56khQ2835c2QCteit7FuL_yG+APpj6-4vRRPxztFtkAMA@mail.gmail.com>
Hi Benjamin,
On Thu, 24 May 2018 13:30:14 +0200
Benjamin Lindqvist <benjamin.lindqvist@endian.se> wrote:
> Hi Stefan,
>
> It seems to me that a probe similar to what the BootROM does shouldn't
> be awfully complicated to implement - just cycle through the switch
> cases in case of an ECC error. But I guess that's more of an idea for
> further improvements rather than a comment to the patch set under
> review.
Nope, not really an option, because you're not guaranteed that the NAND
will be used as a boot media, and the first page or first set of pages
might just be erased.
>
> However, I think that allowing for an override of the oobsize
> inference would be a good idea before merging, no? This could just be
> a trivial #ifdef (at least temporarily). If you agree but don't feel
> like doing it yourself, I'd be happy to pitch in. Let me know.
That's why we have nand-ecc-xxx properties in the DT.
Regards,
Boris
^ permalink raw reply
* Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Ilias Apalodimas @ 2018-05-24 11:50 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: xieyisheng1@huawei.com, kvm@vger.kernel.org,
linux-pci@vger.kernel.org, xuzaibo@huawei.com,
jonathan.cameron@huawei.com, Will Deacon, okaya@codeaurora.org,
linux-mm@kvack.org, yi.l.liu@intel.com, ashok.raj@intel.com,
tn@semihalf.com, joro@8bytes.org, robdclark@gmail.com,
bharatku@xilinx.com, linux-acpi@vger.kernel.org,
liudongdong3@huawei.com, rfranz@cavium.com,
devicetree@vger.kernel.org,
"kevin.tian@intel.com" <kevin.tia>
In-Reply-To: <f73b4a0e-669e-8483-88d7-1b2c8a2b9934@arm.com>
> Interesting, I hadn't thought about this use-case before. At first I
> thought you were talking about mdev devices assigned to VMs, but I think
> you're referring to mdevs assigned to userspace drivers instead? Out of
> curiosity, is it only theoretical or does someone actually need this?
There has been some non upstreamed efforts to have mdev and produce userspace
drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
we did a proof of concept for ethernet interfaces. At the time we choose not to
involve the IOMMU for the reason you mentioned, but having it there would be
good.
Thanks
Ilias
^ permalink raw reply
* Re: [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Jean-Philippe Brucker @ 2018-05-24 11:44 UTC (permalink / raw)
To: Xu Zaibo, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
kvm-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA, ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
rfranz-YGCgFSpz5w/QT0dZR+AlfA, liguozhu,
christian.koenig-5C7GfCeVMHo
In-Reply-To: <5B0536A3.1000304-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Hi,
On 23/05/18 10:38, Xu Zaibo wrote:
>> +static int vfio_iommu_bind_group(struct vfio_iommu *iommu,
>> + struct vfio_group *group,
>> + struct vfio_mm *vfio_mm)
>> +{
>> + int ret;
>> + bool enabled_sva = false;
>> + struct vfio_iommu_sva_bind_data data = {
>> + .vfio_mm = vfio_mm,
>> + .iommu = iommu,
>> + .count = 0,
>> + };
>> +
>> + if (!group->sva_enabled) {
>> + ret = iommu_group_for_each_dev(group->iommu_group, NULL,
>> + vfio_iommu_sva_init);
> Do we need to do *sva_init here or do anything to avoid repeated
> initiation?
> while another process already did initiation at this device, I think
> that current process will get an EEXIST.
Right, sva_init() must be called once for any device that intends to use
bind(). For the second process though, group->sva_enabled will be true
so we won't call sva_init() again, only bind().
Thanks,
Jean
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* Re: [PATCH v2 07/40] iommu: Add a page fault handler
From: Jean-Philippe Brucker @ 2018-05-24 11:44 UTC (permalink / raw)
To: Jacob Pan
Cc: bharatku-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Will Deacon,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
christian.koenig-5C7GfCeVMHo@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20180522163521.413e60c6@jacob-builder>
On 23/05/18 00:35, Jacob Pan wrote:
>>>> + /* Insert *before* the last fault */
>>>> + list_move(&fault->head, &group->faults);
>>>> + }
>>>> +
>>> If you already sorted the group list with last fault at the end,
>>> why do you need a separate entry to track the last fault?
>>
>> Not sure I understand your question, sorry. Do you mean why the
>> iopf_group.last_fault? Just to avoid one more kzalloc.
>>
> kind of :) what i thought was that why can't the last_fault naturally
> be the last entry in your group fault list? then there is no need for
> special treatment in terms of allocation of the last fault. just my
> preference.
But we need a kzalloc for the last fault anyway, so I thought I'd just
piggy-back on the group allocation. And if I don't add the last fault at
the end of group->faults, then it's iopf_handle_group that requires
special treatment. I'm still not sure I understood your question though,
could you send me a patch that does it?
>>>> +
>>>> + queue->flush(queue->flush_arg, dev);
>>>> +
>>>> + /*
>>>> + * No need to clear the partial list. All PRGs containing
>>>> the PASID that
>>>> + * needs to be decommissioned are whole (the device driver
>>>> made sure of
>>>> + * it before this function was called). They have been
>>>> submitted to the
>>>> + * queue by the above flush().
>>>> + */
>>> So you are saying device driver need to make sure LPIG PRQ is
>>> submitted in the flush call above such that partial list is
>>> cleared?
>>
>> Not exactly, it's the IOMMU driver that makes sure all LPIG in its
>> queues are submitted by the above flush call. In more details the
>> flow is:
>>
>> * Either device driver calls unbind()/sva_device_shutdown(), or the
>> process exits.
>> * If the device driver called, then it already told the device to stop
>> using the PASID. Otherwise we use the mm_exit() callback to tell the
>> device driver to stop using the PASID.
>> * In either case, when receiving a stop request from the driver, the
>> device sends the LPIGs to the IOMMU queue.
>> * Then, the flush call above ensures that the IOMMU reports the LPIG
>> with iommu_report_device_fault.
>> * While submitting all LPIGs for this PASID to the work queue,
>> ipof_queue_fault also picked up all partial faults, so the partial
>> list is clean.
>>
>> Maybe I should improve this comment?
>>
> thanks for explaining. LPIG submission is done by device asynchronously
> w.r.t. driver stopping/decommission PASID.
Hmm, it should really be synchronous, otherwise there is no way to know
when the PASID can be decommissioned. We need a guarantee such as the
one in 6.20.1 of the PCIe spec, "Managing PASID TLP Prefix Usage":
"When the stop request mechanism indicates completion, the Function has:
* Completed all Non-Posted Requests associated with this PASID.
* Flushed to the host all Posted Requests addressing host memory in all
TCs that were used by the PASID."
That's in combination with "The function shall [...] finish transmitting
any multi-page Page Request Messages for this PASID (i.e. send the Page
Request Message with the L bit Set)." from the ATS spec.
(If I remember correctly a PRI Page Request is a Posted Request.) Only
after this stop request completes can the driver call unbind(), or
return from exit_mm(). Then we know that if there was a LPIG for that
PASID, it is in the IOMMU's PRI queue (or already completed) once we
call flush().
> so if we were to use this
> flow on vt-d, which does not stall page request queue, then we should
> use the iommu model specific flush() callback to ensure LPIG is
> received? There could be queue full condition and retry. I am just
> trying to understand how and where we can make sure LPIG is
> received and all groups are whole.
For SMMU in patch 30, the flush() callback waits until the PRI queue is
empty or, when the PRI thread is running in parallel, until the thread
has done a full circle (handled as many faults as the queue size). It's
really unpleasant to implement because the flush() callback takes a lock
to inspect the hardware state, but I don't think we have a choice.
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
From: Jean-Philippe Brucker @ 2018-05-24 11:44 UTC (permalink / raw)
To: Jacob Pan
Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, Will Deacon,
okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
bharatku-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rgummal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
christian.koenig-5C7GfCeVMHo@public.gmane.org
In-Reply-To: <20180522094334.71f0e36b@jacob-builder>
On 22/05/18 17:43, Jacob Pan wrote:
> On Thu, 17 May 2018 11:02:42 +0100
> Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
>
>> On 17/05/18 00:31, Jacob Pan wrote:
>>> On Fri, 11 May 2018 20:06:04 +0100
>>> I am a little confused about domain vs. pasid relationship. If
>>> each domain represents a address space, should there be a domain for
>>> each pasid?
>>
>> I don't think there is a formal definition, but from previous
>> discussion the consensus seems to be: domains are a collection of
>> devices that have the same virtual address spaces (one or many).
>>
>> Keeping that definition makes things easier, in my opinion. Some time
>> ago, I did try to represent PASIDs using "subdomains" (introducing a
>> hierarchy of struct iommu_domain), but it required invasive changes in
>> the IOMMU subsystem and probably all over the tree.
>>
>> You do need some kind of "root domain" for each device, so that
>> "iommu_get_domain_for_dev()" still makes sense. That root domain
>> doesn't have a single address space but a collection of subdomains.
>> If you need this anyway, representing a PASID with an iommu_domain
>> doesn't seem preferable than using a different structure (io_mm),
>> because they don't have anything in common.
>>
> My main concern is the PASID table storage. If PASID table storage
> is tied to domain, it is ok to scale up, i.e. multiple devices in a
> domain share a single PASID table. But to scale down, e.g. further
> partition device with VFIO mdev for assignment, each mdev may get its
> own domain via vfio. But there is no IOMMU storage for PASID table at
> mdev device level. Perhaps we do need root domain or some parent-child
> relationship to locate PASID table.
Interesting, I hadn't thought about this use-case before. At first I
thought you were talking about mdev devices assigned to VMs, but I think
you're referring to mdevs assigned to userspace drivers instead? Out of
curiosity, is it only theoretical or does someone actually need this?
I don't think mdev for VM assignment are compatible with PASID, at least
not when the IOMMU is involved. I usually ignore mdev in my reasoning
because, as far as I know, currently they only affect devices that have
their own MMU, and IOMMU domains don't come into play. However, if a
device was backed by the IOMMU, and the device driver wanted to
partition it into mdevs, then users would be tempted to assign mdev1 to
VM1 and mdev2 to VM2.
It doesn't work with PASID, because the PASID spaces of VM1 and VM2 will
conflict. If both VM1 and VM2 allocate PASID #1, then the host has to
somehow arbitrate device accesses, for example scheduling first mdev1
then mdev2. That's possible if the device driver is in charge of the
MMU, but not really compatible with the IOMMU.
So in the IOMMU subsystem, for assigning devices to VMs the only
model that makes sense is SR-IOV, where each VF/mdev has its own RID and
its own PASID table. In that case you'd get one IOMMU domain per VF.
But considering userspace drivers in the host alone, it might make sense
to partition a device into mdevs and assign them to multiple processes.
Interestingly this scheme still doesn't work with the classic MAP/UNMAP
ioctl: since there is a single device context entry for all mdevs, the
mediating driver would have to catch all MAP/UNMAP ioctls and reject
those with IOVAs that overlap those of another mdev. It's doesn't seem
viable. But when using PASID then each mdev has its own address space,
and since PASIDs are allocated by the kernel there is no such conflict.
Anyway, I think this use-case can work with the current structures, if
mediating driver does the bind() instead of VFIO. That's necessary
because you can't let userspace program the PASID into the device, or
they would be able to access address spaces owned by other mdevs. Then
the mediating driver does the bind(), and keeps internal structures to
associate the process to the given mdev.
Thanks,
Jean
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Benjamin Lindqvist @ 2018-05-24 11:30 UTC (permalink / raw)
To: Stefan Agner
Cc: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
mark.rutland, thierry.reding, mturquette, sboyd, Lucas Stach,
miquel.raynal, richard, marcel, krzk, digetx, jonathanh,
pdeschrijver, pgaikwad, Mirza Krak, linux-mtd, linux-tegra,
devicetree, linux-kernel, linux-clk
In-Reply-To: <d8d6cb9fcd8ce7ea5c13b227012de6d2@agner.ch>
Hi Stefan,
It seems to me that a probe similar to what the BootROM does shouldn't
be awfully complicated to implement - just cycle through the switch
cases in case of an ECC error. But I guess that's more of an idea for
further improvements rather than a comment to the patch set under
review.
However, I think that allowing for an override of the oobsize
inference would be a good idea before merging, no? This could just be
a trivial #ifdef (at least temporarily). If you agree but don't feel
like doing it yourself, I'd be happy to pitch in. Let me know.
Best regards,
Benjamin
2018-05-24 13:00 GMT+02:00 Stefan Agner <stefan@agner.ch>:
> On 24.05.2018 09:45, Benjamin Lindqvist wrote:
>> Hi Stefan (and all),
>>
>> First off, I apoloigize in advance if I'm deviating from common
>> kernel mailing list courtesy -- this is my first time responding.
>> I just have a comment on the NAND driver that I'd like to bring
>> to the public.
>>
>
> Welcome!
>
>>> + switch (mtd->oobsize) {
>>> ...
>>> + case 224:
>>> + mtd_set_ooblayout(mtd, &tegra_nand_oob_224_ops);
>>> + chip->ecc.strength = 8;
>>> + chip->ecc.bytes = 18;
>>> + value |= CFG_ECC_SEL | CFG_TVAL_8;
>>> + break; + case 224:
>>
>> I am not sure how you arrived at this oobsize-based inference. I
>> have not seen any explicit relation between oob size and ECC
>> algorithm used in the reference manual. Indeed, the U-Boot I was
>> working on (a fork of the Toradex 2015.04 U-Boot) always has
>> oobsize == 224 but used either BCH[t=16] or RS[t=4]. In fact, we
>> tried choosing RS[t=8] in U-Boot but we failed to make the
>> BootROM decode this at all. So we had to use RS[t=4]. But
>> changing the algorithm did not automatically change the oobsize,
>> at least it didn't for us. So maybe you should consider if this
>> is really the way to go about deciding which algorithm is used.
>
> The oobsize based inference to set the HW ECC mode comes from the
> patchset I picked up. Typically, the size of the OOB area is such that
> it allows "good enough" error correction required for that chip. So
> using it as indicator for the ECC algorithm is not entirely off...
>
> But yeah I agree we have better means, and I already started working on
> a better mechanism. Also, I worked on BCH support, and it looks pretty
> good already.
>
> If we want to auto select mode we can use the ECC requirements from
> ONFI/JEDEC/parameter page. Or we could use device tree only.
>
> Thanks for bringing up the Boot ROM issue. In fact I investigated the
> supported modes the recent days too. First off, as you mentioned, the
> boot ROM seems to probe different modes until it succeeds. By trying
> through all RS and BCH modes, it seems that it only probes some modes:
> - RS t=4
> - BCH t=8
> - BCH t=16
>
> I guess those are preferred modes in practise. Not sure if we should
> make sure the auto selection such that it only chooses from this list...
>
>>
>> Note that we're in fact using this patch set in Linux today, but
>> we had to remove the oobsize inference part. Currently we're
>> simply hard coding it to CFG_TVAL_4, but maybe it would be
>> cleaner to add ECC algo as a board config instead, e.g. in the
>> .dts file or whatever. This seems to be done by other vendors
>> already, see for example excerpt of
>> Documentation/devicetree/bindings/mtd/gpmc-nand.txt below:
>>
>> - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
>> "sw" 1-bit Hamming ecc code via software
>> "hw" <deprecated> use "ham1" instead
>> "hw-romcode" <deprecated> use "ham1" instead
>> "ham1" 1-bit Hamming ecc code
>> "bch4" 4-bit BCH ecc code
>> "bch8" 8-bit BCH ecc code
>> "bch16" 16-bit BCH ECC code
>> Refer below "How to select correct ECC scheme for your device ?"
>>
>> It seems as if this method would be equally applicable to Tegra NAND.
>
> Yeah, ideally we can reuse "nand-ecc-algo". Although, Reed-Solomon is
> not yet in the list. So using this property would require to extend
> this standard property.
>
> --
> Stefan
^ permalink raw reply
* Re: [PATCH v6 05/17] media: rkisp1: add Rockchip ISP1 subdev driver
From: Baruch Siach @ 2018-05-24 11:30 UTC (permalink / raw)
To: Tomasz Figa
Cc: Jacob Chen, ARM/Rockchip SoC, Linux Kernel Mailing List,
IOMMU DRIVERS, Joerg Roedel, linux-arm-kernel,
Mauro Carvalho Chehab, Linux Media Mailing List, Sakari Ailus,
Hans Verkuil, Shunqian Zheng, Laurent Pinchart,
钟以崇, Eddie Cai, Jeffy, devicetree,
Heiko Stübner
In-Reply-To: <CAAFQd5A7gAK4fXH9YVrobR5_QX_5f8xa272R_56v5YUghV6Sxw@mail.gmail.com>
Hi Tomasz,
On Mon, May 07, 2018 at 06:41:50AM +0000, Tomasz Figa wrote:
> On Mon, May 7, 2018 at 3:38 PM Baruch Siach <baruch@tkos.co.il> wrote:
> > On Mon, May 07, 2018 at 06:13:27AM +0000, Tomasz Figa wrote:
> > > On Thu, May 3, 2018 at 6:09 PM Baruch Siach <baruch@tkos.co.il> wrote:
> > > > On Thu, Mar 08, 2018 at 05:47:55PM +0800, Jacob Chen wrote:
> > > > > +static int rkisp1_isp_sd_s_power(struct v4l2_subdev *sd, int on)
> > > > > +{
> > > > > + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> > > > > + int ret;
> > > > > +
> > > > > + v4l2_dbg(1, rkisp1_debug, &isp_dev->v4l2_dev, "s_power: %d\n",
> > > on);
> > > > > +
> > > > > + if (on) {
> > > > > + ret = pm_runtime_get_sync(isp_dev->dev);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + rkisp1_config_clk(isp_dev);
> > > > > + } else {
> > > > > + ret = pm_runtime_put(isp_dev->dev);
> > >
> > > > I commented this line out to make more than one STREAMON work.
> Otherwise,
> > > > the second STREAMON hangs. I guess the bug is not this driver.
> Probably
> > > > something in drivers/soc/rockchip/pm_domains.c. Just noting that in
> case
> > > > you or someone on Cc would like to investigate it further.
> > > >
> > > > I tested v4.16-rc4 on the Tinkerboard.
> > >
> > > Looks like that version doesn't include the IOMMU PM and clock handling
> > > rework [1], which should fix a lot of runtime PM issues. FWIW,
> linux-next
> > > seems to already include it.
> > >
> > > [1] https://lkml.org/lkml/2018/3/23/44
>
> > Thanks for the reference.
>
> > It looks like the iommu driver part is in Linus' tree already. The DT
> part is
> > in the v4.18-armsoc/dts32 branch of Heiko's tree. Am I missing anything?
>
> You're right, most of the series made it in time for 4.17. However, the DT
> part (precisely, the clocks properties added to IOMMU nodes) is crucial for
> the fixes to be effective.
>
> > Anyway, I'll take a look.
>
> Thanks for testing. :) (Forgot to mention in my previous email...)
I finally got around to testing. Unfortunately, kernel v4.17-rc6 with
cherry-pick of commit c78751f91c0b (ARM: dts: rockchip: add clocks in iommu
nodes) from Heiko's tree still exhibit the same problem. STREAMON hangs on
second try. The same workaround "fixes" it.
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH v12 0/8] Clock for CPU scaling support for msm8996
From: Amit Kucheria @ 2018-05-24 11:25 UTC (permalink / raw)
To: Ilia Lin
Cc: Michael Turquette, sboyd, Rob Herring, Mark Rutland, Andy Gross,
David Brown, will.deacon, devicetree, linux-arm-msm, LKML,
vireshk, linux-soc, linux-clk, lakml
In-Reply-To: <1527148218-16540-1-git-send-email-ilialin@codeaurora.org>
On Thu, May 24, 2018 at 10:50 AM, Ilia Lin <ilialin@codeaurora.org> wrote:
> [v12]
> * Addressed the kbuild fail on arm architecture
>
> [v11]
> * Split the series into domains
>
> [v9]
> * Addressed comments from Viresh and Russel about the error handling
>
> [v8]
> * Reordered the patch series into 4 groups
> * Addressed comments from Amit about the comments and commit messages
> * Addressed comments from Amit and Viresh about the resourses deallocation
>
> [v7]
> * Addressed comments from Viresh about resourses deallocation
> and DT compatible
>
> [v6]
> * Addressed comments from Viresh about:
> ** Comments style
> ** Kconfig bool instead of tristate
> ** DT and documentation style
> ** Resourses deallocation on an error
> ** Typos
>
> [v5]
> * Rebased
> * Addressed comments from Bjorn about SPDX style,
> functions and parameters naming
> * Addressed comments from Viresh DT properties and style, comments style,
> resourses deallocation, documentation placement
> * Addressed comments from Sricharan about unnessesary include
> * Addressed comments from Nicolas
> * Addressed comments from Rob about the commit messages and acks
> * Addressed comments from Mark
>
> [v4]
> * Adressed all comments from Stephen
> * Added CPU regulator support
> * Added qcom-cpufreq-kryo driver
>
> [v3]
> * Rebased on top of the latest PLL driver changes
> * Addressed comment from Rob Herring for bindings
>
> [v2]
> * Addressed comments from Rob Herring for bindings
> * Addressed comments from Mark Rutland for memory barrier
> * Addressed comments from Julien Thierry for clock reenabling condition
> * Tuned the HW configuration for clock frequencies below 600MHz
>
> SOC (1/15):
> Extracts the kryo l2 accessors driver from the QCOM PMU driver
>
> Clocks (2/15-9/15):
> This series adds support for the CPU clocks on msm8996 devices.
> The driver uses the existing PLL drivers and is required to control
> the CPU frequency scaling on the MSM8996.
>
> Ilia Lin (6):
> soc: qcom: Separate kryo l2 accessors from PMU driver
> clk: Use devm_ in the register fixed factor clock
> clk: qcom: Add CPU clock driver for msm8996
> dt-bindings: clk: qcom: Add bindings for CPU clock for msm8996
> clk: qcom: cpu-8996: Add support to switch below 600Mhz
> clk: qcom: Add ACD path to CPU clock driver for msm8996
>
> Rajendra Nayak (2):
> clk: qcom: Make clk_alpha_pll_configure available to modules
> clk: qcom: cpu-8996: Add support to switch to alternate PLL
>
> .../devicetree/bindings/clock/qcom,kryocc.txt | 17 +
> drivers/clk/clk-fixed-factor.c | 2 +-
> drivers/clk/qcom/Kconfig | 10 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/clk-alpha-pll.c | 1 +
> drivers/clk/qcom/clk-alpha-pll.h | 6 +
> drivers/clk/qcom/clk-cpu-8996.c | 510 +++++++++++++++++++++
> drivers/perf/Kconfig | 1 +
> drivers/perf/qcom_l2_pmu.c | 90 +---
> drivers/soc/qcom/Kconfig | 3 +
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/kryo-l2-accessors.c | 56 +++
> include/soc/qcom/kryo-l2-accessors.h | 12 +
For the perf/l2-accessors part, Reviewed-by: Amit Kucheria
<amit.kucheria@linaro.org>
For the entire series, Tested-by: Amit Kucheria <amit.kucheria@linaro.org>
^ permalink raw reply
* Re: [PATCH v12 0/2] Kryo CPU scaling driver
From: Amit Kucheria @ 2018-05-24 11:20 UTC (permalink / raw)
To: Ilia Lin
Cc: vireshk, nm, sboyd, Rob Herring, Mark Rutland, Rafael J. Wysocki,
Linux PM list, devicetree, LKML
In-Reply-To: <1527152242-31281-1-git-send-email-ilialin@codeaurora.org>
On Thu, May 24, 2018 at 11:57 AM, Ilia Lin <ilialin@codeaurora.org> wrote:
> [v12]
> * Addressed comments from Sudeep and Viresh about the single init
>
> [v11]
> * Addressed comment from Russel about device_node reference
> * Addressed comment from Sudeep about the late_initcall
> * Transformed init into probe to take care of deferals
>
> [v10]
> * Split the series into domains
> * Addressed comments from Viresh and Sudeep about logical CPU numbering.
>
> The qcom-cpufreq-kryo driver is aimed to support different SOC versions.
> The driver reads eFuse information and chooses the required OPP subset
> by passing the OPP supported-hw parameter.
>
> The series depends on the series from Viresh:
> https://patchwork.kernel.org/patch/10418139/
>
> The previous spin was here:
> https://patchwork.kernel.org/patch/10421143/
>
> Ilia Lin (2):
> cpufreq: Add Kryo CPU scaling driver
> dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu
>
> .../devicetree/bindings/opp/kryo-cpufreq.txt | 680 +++++++++++++++++++++
> drivers/cpufreq/Kconfig.arm | 10 +
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/cpufreq-dt-platdev.c | 3 +
> drivers/cpufreq/qcom-cpufreq-kryo.c | 194 ++++++
> 5 files changed, 888 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
>
> --
> 1.9.1
>
For this series,
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Tested-by: Amit Kucheria <amit.kucheria@linaro.org>
^ permalink raw reply
* [PATCH v4] arm64: allwinner: a64: Add Amarula A64-Relic initial support
From: Jagan Teki @ 2018-05-24 11:16 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Michael Trimarchi, Icenowy Zheng
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Jagan Teki
Amarula A64-Relic is Allwinner A64 based IoT device, which support
- Allwinner A64 Cortex-A53
- Mali-400MP2 GPU
- AXP803 PMIC
- 1GB DDR3 RAM
- 8GB eMMC
- AP6330 Wifi/BLE
- MIPI-DSI
- CSI: OV5640 sensor
- USB OTG
- 12V DC power supply
Signed-off-by: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
---
Changes for v4:
- update dr_mode as 'otg'
Changes for v3:
- Use sun50i-a64-amarula-relic.dts name
- add eldo3 for dvdd-csi
- update dldo4 min voltage as 3.3v as per schematics
- use dldo3 name as dovdd-csi
- update aldo1, aldo2 voltages as per schematics
Changes for v2:
- Rename dts name to sun50i-a64-relic.dts which is simple to use
- Update dldo4 min voltage as 1.8
- Use licence year as 2018
arch/arm64/boot/dts/allwinner/Makefile | 1 +
.../dts/allwinner/sun50i-a64-amarula-relic.dts | 188 +++++++++++++++++++++
2 files changed, 189 insertions(+)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index c31f90a49481..67ce8c500b2e 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-amarula-relic.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-bananapi-m64.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-nanopi-a64.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-olinuxino.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
new file mode 100644
index 000000000000..ce4a256ff086
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (C) 2018 Amarula Solutions B.V.
+ * Author: Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
+ */
+
+/dts-v1/;
+
+#include "sun50i-a64.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+ model = "Amarula A64-Relic";
+ compatible = "amarula,a64-relic", "allwinner,sun50i-a64";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+};
+
+&ehci0 {
+ status = "okay";
+};
+
+&mmc2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc2_pins>;
+ vmmc-supply = <®_dcdc1>;
+ bus-width = <8>;
+ non-removable;
+ cap-mmc-hw-reset;
+ status = "okay";
+};
+
+&ohci0 {
+ status = "okay";
+};
+
+&r_rsb {
+ status = "okay";
+
+ axp803: pmic@3a3 {
+ compatible = "x-powers,axp803";
+ reg = <0x3a3>;
+ interrupt-parent = <&r_intc>;
+ interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ x-powers,drive-vbus-en; /* set N_VBUSEN as output pin */
+ };
+};
+
+#include "axp803.dtsi"
+
+®_aldo1 {
+ regulator-always-on;
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "avdd-csi";
+};
+
+®_aldo2 {
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-pl";
+};
+
+®_aldo3 {
+ regulator-always-on;
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-name = "vcc-pll-avcc";
+};
+
+®_dcdc1 {
+ regulator-always-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-3v3";
+};
+
+®_dcdc2 {
+ regulator-always-on;
+ regulator-min-microvolt = <1040000>;
+ regulator-max-microvolt = <1300000>;
+ regulator-name = "vdd-cpux";
+};
+
+/* DCDC3 is polyphased with DCDC2 */
+
+®_dcdc5 {
+ regulator-always-on;
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-name = "vcc-dram";
+};
+
+®_dcdc6 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vdd-sys";
+};
+
+®_dldo1 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-hdmi-dsi-sensor";
+};
+
+®_dldo2 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-mipi";
+};
+
+®_dldo3 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "dovdd-csi";
+};
+
+®_dldo4 {
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-wifi-io";
+};
+
+®_drivevbus {
+ regulator-name = "usb0-vbus";
+ status = "okay";
+};
+
+®_eldo1 {
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "cpvdd";
+};
+
+®_eldo3 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "dvdd-csi";
+};
+
+®_fldo1 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "vcc-1v2-hsic";
+};
+
+/*
+ * The A64 chip cannot work without this regulator off, although
+ * it seems to be only driving the AR100 core.
+ * Maybe we don't still know well about CPUs domain.
+ */
+®_fldo2 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vdd-cpus";
+};
+
+®_rtc_ldo {
+ regulator-name = "vcc-rtc";
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pins_a>;
+ status = "okay";
+};
+
+&usb_otg {
+ dr_mode = "otg";
+ status = "okay";
+};
+
+&usbphy {
+ usb0_id_det-gpios = <&pio 7 9 GPIO_ACTIVE_HIGH>; /* PH9 */
+ usb0_vbus-supply = <®_drivevbus>;
+ status = "okay";
+};
--
2.14.3
^ permalink raw reply related
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-05-24 11:09 UTC (permalink / raw)
To: Boris Brezillon
Cc: dwmw2, computersforpeace, marek.vasut, robh+dt, mark.rutland,
thierry.reding, mturquette, sboyd, dev, miquel.raynal, richard,
marcel, krzk, digetx, benjamin.lindqvist, jonathanh, pdeschrijver,
pgaikwad, mirza.krak, linux-mtd, linux-tegra, devicetree,
linux-kernel, linux-clk
In-Reply-To: <20180524105614.3c51736c@bbrezillon>
On 24.05.2018 10:56, Boris Brezillon wrote:
> On Thu, 24 May 2018 10:46:27 +0200
> Stefan Agner <stefan@agner.ch> wrote:
>
>> Hi Boris,
>>
>> Thanks for the initial review! One small question below:
>>
>> On 23.05.2018 16:18, Boris Brezillon wrote:
>> > Hi Stefan,
>> >
>> > On Tue, 22 May 2018 14:07:06 +0200
>> > Stefan Agner <stefan@agner.ch> wrote:
>> >> +
>> >> +struct tegra_nand {
>> >> + void __iomem *regs;
>> >> + struct clk *clk;
>> >> + struct gpio_desc *wp_gpio;
>> >> +
>> >> + struct nand_chip chip;
>> >> + struct device *dev;
>> >> +
>> >> + struct completion command_complete;
>> >> + struct completion dma_complete;
>> >> + bool last_read_error;
>> >> +
>> >> + dma_addr_t data_dma;
>> >> + void *data_buf;
>> >> + dma_addr_t oob_dma;
>> >> + void *oob_buf;
>> >> +
>> >> + int cur_chip;
>> >> +};
>> >
>> > This struct should be split in 2 structures: one representing the NAND
>> > controller and one representing the NAND chip:
>> >
>> > struct tegra_nand_controller {
>> > struct nand_hw_control base;
>> > void __iomem *regs;
>> > struct clk *clk;
>> > struct device *dev;
>> > struct completion command_complete;
>> > struct completion dma_complete;
>> > bool last_read_error;
>> > int cur_chip;
>> > };
>> >
>> > struct tegra_nand {
>> > struct nand_chip base;
>> > dma_addr_t data_dma;
>> > void *data_buf;
>> > dma_addr_t oob_dma;
>> > void *oob_buf;
>> > };
>>
>> Is there a particular reason why you would leave DMA buffers in the chip
>> structure? It seems that is more a controller thing...
>
> The size of those buffers is likely to be device dependent, so if you
> have several NANDs connected to the controller, you'll either have to
> have one buffer at the controller level which is max(all-chip-buf-size)
> or a buffer per device.
>
> Also, do you really need these buffers? The core already provide some
> which are suitable for DMA (chip->oob_poi and chip->data_buf).
>
Good question, I am not sure, that was existing code.
Are you sure data_buf it is DMA capable?
nand_scan_tail allocates with kmalloc:
chip->data_buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
--
Stefan
>>
>> If I move them, then struct tegra_nand would be basically empty. Can I
>> just use struct nand_chip and have no driver specific chip abstraction?
>
> Sure.
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Boris Brezillon @ 2018-05-24 11:07 UTC (permalink / raw)
To: Stefan Agner
Cc: Benjamin Lindqvist, dwmw2, computersforpeace, marek.vasut,
robh+dt, mark.rutland, thierry.reding, mturquette, sboyd,
Lucas Stach, miquel.raynal, richard, marcel, krzk, digetx,
jonathanh, pdeschrijver, pgaikwad, Mirza Krak, linux-mtd,
linux-tegra, devicetree, linux-kernel, linux-clk
In-Reply-To: <d8d6cb9fcd8ce7ea5c13b227012de6d2@agner.ch>
On Thu, 24 May 2018 13:00:41 +0200
Stefan Agner <stefan@agner.ch> wrote:
>
> >
> > Note that we're in fact using this patch set in Linux today, but
> > we had to remove the oobsize inference part. Currently we're
> > simply hard coding it to CFG_TVAL_4, but maybe it would be
> > cleaner to add ECC algo as a board config instead, e.g. in the
> > .dts file or whatever. This seems to be done by other vendors
> > already, see for example excerpt of
> > Documentation/devicetree/bindings/mtd/gpmc-nand.txt below:
> >
> > - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
> > "sw" 1-bit Hamming ecc code via software
> > "hw" <deprecated> use "ham1" instead
> > "hw-romcode" <deprecated> use "ham1" instead
> > "ham1" 1-bit Hamming ecc code
> > "bch4" 4-bit BCH ecc code
> > "bch8" 8-bit BCH ecc code
> > "bch16" 16-bit BCH ECC code
> > Refer below "How to select correct ECC scheme for your device ?"
> >
> > It seems as if this method would be equally applicable to Tegra NAND.
>
> Yeah, ideally we can reuse "nand-ecc-algo". Although, Reed-Solomon is
> not yet in the list. So using this property would require to extend
> this standard property.
I'm okay with that ;-).
^ permalink raw reply
* Re: [RESEND PATCH 2/5] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
From: Stefan Agner @ 2018-05-24 11:00 UTC (permalink / raw)
To: Benjamin Lindqvist
Cc: boris.brezillon, dwmw2, computersforpeace, marek.vasut, robh+dt,
mark.rutland, thierry.reding, mturquette, sboyd, Lucas Stach,
miquel.raynal, richard, marcel, krzk, digetx, jonathanh,
pdeschrijver, pgaikwad, Mirza Krak, linux-mtd, linux-tegra,
devicetree, linux-kernel, linux-clk
In-Reply-To: <CAGefG55kvShaLhNYsFznbOq0_6RNbb+eJ47DLnnU9qRRpwu-iA@mail.gmail.com>
On 24.05.2018 09:45, Benjamin Lindqvist wrote:
> Hi Stefan (and all),
>
> First off, I apoloigize in advance if I'm deviating from common
> kernel mailing list courtesy -- this is my first time responding.
> I just have a comment on the NAND driver that I'd like to bring
> to the public.
>
Welcome!
>> + switch (mtd->oobsize) {
>> ...
>> + case 224:
>> + mtd_set_ooblayout(mtd, &tegra_nand_oob_224_ops);
>> + chip->ecc.strength = 8;
>> + chip->ecc.bytes = 18;
>> + value |= CFG_ECC_SEL | CFG_TVAL_8;
>> + break; + case 224:
>
> I am not sure how you arrived at this oobsize-based inference. I
> have not seen any explicit relation between oob size and ECC
> algorithm used in the reference manual. Indeed, the U-Boot I was
> working on (a fork of the Toradex 2015.04 U-Boot) always has
> oobsize == 224 but used either BCH[t=16] or RS[t=4]. In fact, we
> tried choosing RS[t=8] in U-Boot but we failed to make the
> BootROM decode this at all. So we had to use RS[t=4]. But
> changing the algorithm did not automatically change the oobsize,
> at least it didn't for us. So maybe you should consider if this
> is really the way to go about deciding which algorithm is used.
The oobsize based inference to set the HW ECC mode comes from the
patchset I picked up. Typically, the size of the OOB area is such that
it allows "good enough" error correction required for that chip. So
using it as indicator for the ECC algorithm is not entirely off...
But yeah I agree we have better means, and I already started working on
a better mechanism. Also, I worked on BCH support, and it looks pretty
good already.
If we want to auto select mode we can use the ECC requirements from
ONFI/JEDEC/parameter page. Or we could use device tree only.
Thanks for bringing up the Boot ROM issue. In fact I investigated the
supported modes the recent days too. First off, as you mentioned, the
boot ROM seems to probe different modes until it succeeds. By trying
through all RS and BCH modes, it seems that it only probes some modes:
- RS t=4
- BCH t=8
- BCH t=16
I guess those are preferred modes in practise. Not sure if we should
make sure the auto selection such that it only chooses from this list...
>
> Note that we're in fact using this patch set in Linux today, but
> we had to remove the oobsize inference part. Currently we're
> simply hard coding it to CFG_TVAL_4, but maybe it would be
> cleaner to add ECC algo as a board config instead, e.g. in the
> .dts file or whatever. This seems to be done by other vendors
> already, see for example excerpt of
> Documentation/devicetree/bindings/mtd/gpmc-nand.txt below:
>
> - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
> "sw" 1-bit Hamming ecc code via software
> "hw" <deprecated> use "ham1" instead
> "hw-romcode" <deprecated> use "ham1" instead
> "ham1" 1-bit Hamming ecc code
> "bch4" 4-bit BCH ecc code
> "bch8" 8-bit BCH ecc code
> "bch16" 16-bit BCH ECC code
> Refer below "How to select correct ECC scheme for your device ?"
>
> It seems as if this method would be equally applicable to Tegra NAND.
Yeah, ideally we can reuse "nand-ecc-algo". Although, Reed-Solomon is
not yet in the list. So using this property would require to extend
this standard property.
--
Stefan
^ permalink raw reply
* [PATCH v9 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs
From: Raju P L S S S N @ 2018-05-24 10:45 UTC (permalink / raw)
To: andy.gross, david.brown, linux-arm-msm, linux-soc
Cc: rnayak, bjorn.andersson, linux-kernel, sboyd, evgreen, dianders,
mka, rplsssn, ilina, devicetree
In-Reply-To: <1527158731-17685-1-git-send-email-rplsssn@codeaurora.org>
From: Lina Iyer <ilina@codeaurora.org>
Add device binding documentation for Qualcomm Technology Inc's RPMH RSC
driver. The driver is used for communicating resource state requests for
shared resources.
Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes in v8:
- Describe IRQ for all DRVs
Changes in v7:
- Fix example
Changes in v6:
- Address comments from Stephen Boyd
Changes in v3:
- Move to soc/qcom
- Amend text per Stephen's suggestions
Changes in v2:
- Amend text to describe the registers in reg property
- Add reg-names for the registers
- Update examples to use GIC_SPI in interrupts instead of 0
- Rephrase incorrect description
Changes in v3:
- Fix unwanted capitalization
- Remove clients from the examples, this doc does not describe
them
- Rephrase introductory paragraph
- Remove hardware specifics from DT bindings
---
.../devicetree/bindings/soc/qcom/rpmh-rsc.txt | 137 +++++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
new file mode 100644
index 0000000..e15c100
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
@@ -0,0 +1,137 @@
+RPMH RSC:
+------------
+
+Resource Power Manager Hardened (RPMH) is the mechanism for communicating with
+the hardened resource accelerators on Qualcomm SoCs. Requests to the resources
+can be written to the Trigger Command Set (TCS) registers and using a (addr,
+val) pair and triggered. Messages in the TCS are then sent in sequence over an
+internal bus.
+
+The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
+(Resource State Coordinator a.k.a RSC) that can handle multiple sleep and
+active/wake resource requests. Multiple such DRVs can exist in a SoC and can
+be written to from Linux. The structure of each DRV follows the same template
+with a few variations that are captured by the properties here.
+
+A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
+have powered off to facilitate idle power saving. TCS could be classified as -
+
+ SLEEP /* Triggered by F/W */
+ WAKE /* Triggered by F/W */
+ ACTIVE /* Triggered by Linux */
+ CONTROL /* Triggered by F/W */
+
+The order in which they are described in the DT, should match the hardware
+configuration.
+
+Requests can be made for the state of a resource, when the subsystem is active
+or idle. When all subsystems like Modem, GPU, CPU are idle, the resource state
+will be an aggregate of the sleep votes from each of those subsystems. Clients
+may request a sleep value for their shared resources in addition to the active
+mode requests.
+
+Properties:
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: Should be "qcom,rpmh-rsc".
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: The first register specifies the base address of the
+ DRV(s). The number of DRVs in the dependent on the RSC.
+ The tcs-offset specifies the start address of the
+ TCS in the DRVs.
+
+- reg-names:
+ Usage: required
+ Value type: <string>
+ Definition: Maps the register specified in the reg property. Must be
+ "drv-0", "drv-1", "drv-2" etc and "tcs-offset". The
+
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-interrupt>
+ Definition: The interrupt that trips when a message complete/response
+ is received for this DRV from the accelerators.
+
+- qcom,drv-id:
+ Usage: required
+ Value type: <u32>
+ Definition: The id of the DRV in the RSC block that will be used by
+ this controller.
+
+- qcom,tcs-config:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: The tuple defining the configuration of TCS.
+ Must have 2 cells which describe each TCS type.
+ <type number_of_tcs>.
+ The order of the TCS must match the hardware
+ configuration.
+ - Cell #1 (TCS Type): TCS types to be specified -
+ SLEEP_TCS
+ WAKE_TCS
+ ACTIVE_TCS
+ CONTROL_TCS
+ - Cell #2 (Number of TCS): <u32>
+
+- label:
+ Usage: optional
+ Value type: <string>
+ Definition: Name for the RSC. The name would be used in trace logs.
+
+Drivers that want to use the RSC to communicate with RPMH must specify their
+bindings as child nodes of the RSC controllers they wish to communicate with.
+
+Example 1:
+
+For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
+register offsets for DRV2 start at 0D00, the register calculations are like
+this -
+DRV0: 0x179C0000
+DRV2: 0x179C0000 + 0x10000 = 0x179D0000
+DRV2: 0x179C0000 + 0x10000 * 2 = 0x179E0000
+TCS-OFFSET: 0xD00
+
+ apps_rsc: rsc@179c0000 {
+ label = "apps_rsc";
+ compatible = "qcom,rpmh-rsc";
+ reg = <0x179c0000 0x10000>,
+ <0x179d0000 0x10000>,
+ <0x179e0000 0x10000>;
+ reg-names = "drv-0", "drv-1", "drv-2";
+ interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
+ qcom,tcs-offset = <0xd00>;
+ qcom,drv-id = <2>;
+ qcom,tcs-config = <SLEEP_TCS 3>,
+ <WAKE_TCS 3>,
+ <ACTIVE_TCS 2>,
+ <CONTROL_TCS 1>;
+ };
+
+Example 2:
+
+For a TCS whose RSC base address is 0xAF20000 and is at DRV id of 0, the
+register offsets for DRV0 start at 01C00, the register calculations are like
+this -
+DRV0: 0xAF20000
+TCS-OFFSET: 0x1C00
+
+ disp_rsc: rsc@af20000 {
+ label = "disp_rsc";
+ compatible = "qcom,rpmh-rsc";
+ reg = <0xaf20000 0x10000>;
+ reg-names = "drv-0";
+ interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
+ qcom,tcs-offset = <0x1c00>;
+ qcom,drv-id = <0>;
+ qcom,tcs-config = <SLEEP_TCS 1>,
+ <WAKE_TCS 1>,
+ <ACTIVE_TCS 0>,
+ <CONTROL_TCS 0>;
+ };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH v3 3/3] ARM: dts: Renesas R9A06G032 SMP enable method
From: Michel Pollet @ 2018-05-24 10:39 UTC (permalink / raw)
To: linux-renesas-soc, Simon Horman
Cc: phil.edworthy, Michel Pollet, Michel Pollet, Rob Herring,
Mark Rutland, Magnus Damm, Russell King, Florian Fainelli,
Martin Blumenstingl, Greg Kroah-Hartman, Stefan Wahren,
Rajendra Nayak, Carlo Caione, Chen-Yu Tsai, Frank Rowand,
Juri Lelli, devicetree, linux-kernel, linux-arm-kernel
Add a special enable method for the second CA7 of the R9A06G032
as well as the default value for the "cpu-release-addr" property.
Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
arch/arm/boot/dts/r9a06g032.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 9534f1b..d7b5414 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -30,6 +30,8 @@
compatible = "arm,cortex-a7";
reg = <1>;
clocks = <&sysctrl R9A06G032_DIV_CA7>;
+ enable-method = "renesas,r9a06g032-smp";
+ cpu-release-addr = <0x4000c204>;
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Michel Pollet @ 2018-05-24 10:30 UTC (permalink / raw)
To: linux-renesas-soc, Simon Horman
Cc: phil.edworthy, Michel Pollet, Michel Pollet, Rob Herring,
Mark Rutland, Magnus Damm, Russell King, Chen-Yu Tsai,
Greg Kroah-Hartman, Kevin Hilman, Stefan Wahren, Rajendra Nayak,
Andreas Färber, Juri Lelli, Florian Fainelli, Frank Rowand,
Carlo Caione, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1527157834-7747-1-git-send-email-michel.pollet@bp.renesas.com>
The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time, it
requires a special enable method to get it started.
Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
arch/arm/mach-shmobile/Makefile | 1 +
arch/arm/mach-shmobile/smp-r9a06g032.c | 85 ++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+)
create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 1939f52..d7fc98f 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0) += smp-sh73a0.o headsmp-scu.o platsmp-scu.o
smp-$(CONFIG_ARCH_R8A7779) += smp-r8a7779.o headsmp-scu.o platsmp-scu.o
smp-$(CONFIG_ARCH_R8A7790) += smp-r8a7790.o
smp-$(CONFIG_ARCH_R8A7791) += smp-r8a7791.o
+smp-$(CONFIG_ARCH_R9A06G032) += smp-r9a06g032.o
smp-$(CONFIG_ARCH_EMEV2) += smp-emev2.o headsmp-scu.o platsmp-scu.o
# PM objects
diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c b/arch/arm/mach-shmobile/smp-r9a06g032.c
new file mode 100644
index 0000000..a536e89
--- /dev/null
+++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZ/N1D Second CA7 enabler.
+ *
+ * Copyright (C) 2018 Renesas Electronics Europe Limited
+ *
+ * Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
+ * Derived from action,s500-smp
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+#include <asm/cacheflush.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+/*
+ * The second CPU is parked in ROM at boot time. It requires waking it after
+ * writing an address into the BOOTADDR register of sysctrl.
+ *
+ * So the default value of the "cpu-release-addr" corresponds to BOOTADDR...
+ *
+ * *However* the BOOTADDR register is not available when the kernel
+ * starts in NONSEC mode.
+ *
+ * So for NONSEC mode, the bootloader re-parks the second CPU into a pen
+ * in SRAM, and changes the "cpu-release-addr" of linux's DT to a SRAM address,
+ * which is not restricted.
+ */
+
+static void __iomem *cpu_bootaddr;
+
+static DEFINE_SPINLOCK(cpu_lock);
+
+static int rzn1_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ if (!cpu_bootaddr)
+ return -ENODEV;
+
+ spin_lock(&cpu_lock);
+
+ writel(__pa_symbol(secondary_startup), cpu_bootaddr);
+ arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+ spin_unlock(&cpu_lock);
+
+ return 0;
+}
+
+static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
+{
+ struct device_node *dn;
+ int ret;
+ u32 bootaddr;
+
+ dn = of_get_cpu_node(1, NULL);
+ if (!dn) {
+ pr_err("CPU#1: missing device tree node\n");
+ return;
+ }
+ /*
+ * Determine the address from which the CPU is polling.
+ * The bootloader *does* change this property
+ */
+ ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
+ of_node_put(dn);
+ if (ret) {
+ pr_err("CPU#1: invalid cpu-release-addr property\n");
+ return;
+ }
+ pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
+
+ cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));
+ if (!cpu_bootaddr)
+ pr_err("CPU#1: cpu-release-addr map failed\n");
+}
+
+static const struct smp_operations rzn1_smp_ops __initconst = {
+ .smp_prepare_cpus = rzn1_smp_prepare_cpus,
+ .smp_boot_secondary = rzn1_smp_boot_secondary,
+};
+CPU_METHOD_OF_DECLARE(rzn1_smp, "renesas,r9a06g032-smp", &rzn1_smp_ops);
--
2.7.4
^ 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