Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] arm64: defconfig: Enable sound drivers on Allwinner devices
From: Alistair Francis @ 2019-08-08 21:37 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: alistair23, Alistair Francis, will, catalin.marinas
In-Reply-To: <20190808213718.12270-1-alistair@alistair23.me>

Enable the sound drivers for Allwinner devices.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 arch/arm64/configs/defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b17ed20e1754..3dc12c3b9bf8 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -550,6 +550,12 @@ CONFIG_SND_SOC_ROCKCHIP_RT5645=m
 CONFIG_SND_SOC_RK3399_GRU_SOUND=m
 CONFIG_SND_SOC_SAMSUNG=y
 CONFIG_SND_SOC_RCAR=m
+CONFIG_SND_SUN4I_CODEC=m
+CONFIG_SND_SUN8I_CODEC=m
+CONFIG_SND_SUN8I_CODEC_ANALOG=m
+CONFIG_SND_SUN50I_CODEC_ANALOG=m
+CONFIG_SND_SUN4I_I2S=m
+CONFIG_SND_SUN4I_SPDIF=m
 CONFIG_SND_SOC_AK4613=m
 CONFIG_SND_SOC_ES7134=m
 CONFIG_SND_SOC_ES7241=m
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/2] arm64: defconfig: Cleanup the defconfig
From: Alistair Francis @ 2019-08-08 21:37 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: alistair23, Alistair Francis, will, catalin.marinas

Re-run savedefconfig to cleanup the defconfig.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 arch/arm64/configs/defconfig | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0e58ef02880c..b17ed20e1754 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -88,7 +88,6 @@ CONFIG_ARM_TEGRA186_CPUFREQ=y
 CONFIG_ARM_SCPI_PROTOCOL=y
 CONFIG_RASPBERRYPI_FIRMWARE=y
 CONFIG_INTEL_STRATIX10_SERVICE=y
-CONFIG_TI_SCI_PROTOCOL=y
 CONFIG_EFI_CAPSULE_LOADER=y
 CONFIG_IMX_SCU=y
 CONFIG_IMX_SCU_PD=y
@@ -193,7 +192,6 @@ CONFIG_PCIE_QCOM=y
 CONFIG_PCIE_ARMADA_8K=y
 CONFIG_PCIE_KIRIN=y
 CONFIG_PCIE_HISI_STB=y
-CONFIG_PCIE_TEGRA194=m
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_HISILICON_LPC=y
@@ -260,18 +258,12 @@ CONFIG_HNS3_ENET=y
 CONFIG_E1000E=y
 CONFIG_IGB=y
 CONFIG_IGBVF=y
-CONFIG_MLX4_EN=m
-CONFIG_MLX4_CORE=m
-CONFIG_MLX4_DEBUG=y
-CONFIG_MLX4_CORE_GEN2=y
-CONFIG_MLX5_CORE=m
-CONFIG_MLX5_CORE_EN=y
-CONFIG_MLX5_EN_ARFS=y
-CONFIG_MLX5_EN_RXNFC=y
-CONFIG_MLX5_MPFS=y
 CONFIG_MVNETA=y
 CONFIG_MVPP2=y
 CONFIG_SKY2=y
+CONFIG_MLX4_EN=m
+CONFIG_MLX5_CORE=m
+CONFIG_MLX5_CORE_EN=y
 CONFIG_QCOM_EMAC=m
 CONFIG_RAVB=y
 CONFIG_SMC91X=y
@@ -386,8 +378,8 @@ CONFIG_SPI_PL022=y
 CONFIG_SPI_ROCKCHIP=y
 CONFIG_SPI_QUP=y
 CONFIG_SPI_S3C64XX=y
-CONFIG_SPI_SPIDEV=m
 CONFIG_SPI_SUN6I=y
+CONFIG_SPI_SPIDEV=m
 CONFIG_SPMI=y
 CONFIG_PINCTRL_SINGLE=y
 CONFIG_PINCTRL_MAX77620=y
@@ -593,7 +585,6 @@ CONFIG_USB_GADGET=y
 CONFIG_USB_RENESAS_USBHS_UDC=m
 CONFIG_USB_RENESAS_USB3=m
 CONFIG_TYPEC=m
-CONFIG_TYPEC_HD3SS3220=m
 CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=32
 CONFIG_MMC_ARMMMCI=y
@@ -648,9 +639,9 @@ CONFIG_RTC_DRV_SNVS=m
 CONFIG_RTC_DRV_IMX_SC=m
 CONFIG_RTC_DRV_XGENE=y
 CONFIG_DMADEVICES=y
-CONFIG_FSL_EDMA=y
 CONFIG_DMA_BCM2835=m
 CONFIG_DMA_SUN6I=m
+CONFIG_FSL_EDMA=y
 CONFIG_K3_DMA=y
 CONFIG_MV_XOR=y
 CONFIG_MV_XOR_V2=y
@@ -696,7 +687,6 @@ CONFIG_ARM_MHU=y
 CONFIG_IMX_MBOX=y
 CONFIG_PLATFORM_MHU=y
 CONFIG_BCM2835_MBOX=y
-CONFIG_TI_MESSAGE_MANAGER=y
 CONFIG_QCOM_APCS_IPC=y
 CONFIG_ROCKCHIP_IOMMU=y
 CONFIG_TEGRA_IOMMU_SMMU=y
@@ -736,9 +726,7 @@ CONFIG_ARCH_TEGRA_186_SOC=y
 CONFIG_ARCH_TEGRA_194_SOC=y
 CONFIG_ARCH_K3_AM6_SOC=y
 CONFIG_ARCH_K3_J721E_SOC=y
-CONFIG_SOC_TI=y
 CONFIG_TI_SCI_PM_DOMAINS=y
-CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND=y
 CONFIG_EXTCON_USB_GPIO=y
 CONFIG_EXTCON_USBC_CROS_EC=y
 CONFIG_MEMORY=y
@@ -783,11 +771,11 @@ CONFIG_PHY_TEGRA_XUSB=y
 CONFIG_HISI_PMU=y
 CONFIG_QCOM_L2_PMU=y
 CONFIG_QCOM_L3_PMU=y
-CONFIG_NVMEM_SUNXI_SID=y
 CONFIG_NVMEM_IMX_OCOTP=y
 CONFIG_NVMEM_IMX_OCOTP_SCU=y
 CONFIG_QCOM_QFPROM=y
 CONFIG_ROCKCHIP_EFUSE=y
+CONFIG_NVMEM_SUNXI_SID=y
 CONFIG_UNIPHIER_EFUSE=y
 CONFIG_MESON_EFUSE=m
 CONFIG_FPGA=y
-- 
2.22.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* RE: [EXT] Re: i2c: imx: support slave mode for imx I2C driver
From: Biwen Li @ 2019-08-09  4:04 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: wsa@the-dreams.de, shawnguo@kernel.org,
	linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org,
	kernel@pengutronix.de, Laurentiu Tudor, festevam@gmail.com,
	linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <20190808141814.wr4hlmbaek6p7dy2@pengutronix.de>

> 
> Hi,
> 
> On Thu, Aug 08, 2019 at 11:53:43AM +0800, Biwen Li wrote:
> > The patch supports slave mode for imx I2C driver
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > ---
> >  drivers/i2c/busses/i2c-imx.c | 199
> > ++++++++++++++++++++++++++++++++---
> >  1 file changed, 185 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx.c
> > b/drivers/i2c/busses/i2c-imx.c index b1b8b938d7f4..f7583a9fa56f 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -202,6 +202,9 @@ struct imx_i2c_struct {
> >       struct pinctrl_state *pinctrl_pins_gpio;
> >
> >       struct imx_i2c_dma      *dma;
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +     struct i2c_client               *slave;
> > +#endif /* CONFIG_I2C_SLAVE */
> 
> Other drivers just do a "select I2C_SLAVE" in Kconfig to get rid of these #ifs. We
> should do the same.
Hi sascha, I don't know your meaning, could you let it clearer?
> 
> >  };
> >
> >  static const struct imx_i2c_hwdata imx1_i2c_hwdata = { @@ -583,23
> > +586,40 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
> >       imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);  }
> >
> > -static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
> > +/* Clear interrupt flag bit */
> > +static void i2c_imx_clr_if_bit(struct imx_i2c_struct *i2c_imx)
> >  {
> > -     struct imx_i2c_struct *i2c_imx = dev_id;
> > -     unsigned int temp;
> > +     unsigned int status;
> >
> > -     temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > -     if (temp & I2SR_IIF) {
> > -             /* save status register */
> > -             i2c_imx->i2csr = temp;
> > -             temp &= ~I2SR_IIF;
> > -             temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
> > -             imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
> > -             wake_up(&i2c_imx->queue);
> > -             return IRQ_HANDLED;
> > -     }
> > +     status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +     status &= ~I2SR_IIF;
> > +     status |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
> > +     imx_i2c_write_reg(status, i2c_imx, IMX_I2C_I2SR); }
> > +
> > +/* Clear arbitration lost bit */
> > +static void i2c_imx_clr_al_bit(struct imx_i2c_struct *i2c_imx) {
> > +     unsigned int status;
> > +
> > +     status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +     status &= ~I2SR_IAL;
> > +     imx_i2c_write_reg(status, i2c_imx, IMX_I2C_I2SR); }
> >
> > -     return IRQ_NONE;
> > +static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx)
> > +{
> > +     unsigned int status;
> > +
> > +     dev_dbg(&i2c_imx->adapter.dev, "<%s>: master interrupt\n",
> > + __func__);
> 
> Generally this driver has way too many dev_dbg spread around in hot pathes
> already. IMO adding more doesn't make the output more useful.
Ok, got it. I will delete it in v2.
> 
> > +
> > +     /* Save status register */
> > +     status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +     i2c_imx->i2csr = status | I2SR_IIF;
> > +
> > +     wake_up(&i2c_imx->queue);
> > +
> > +     return IRQ_HANDLED;
> >  }
> >
> >  static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx, @@
> > -1043,11 +1063,162 @@ static u32 i2c_imx_func(struct i2c_adapter
> *adapter)
> >               | I2C_FUNC_SMBUS_READ_BLOCK_DATA;  }
> >
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +static void i2c_imx_slave_init(struct imx_i2c_struct *i2c_imx) {
> > +     unsigned int temp;
> > +
> > +     dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> > +
> > +     /* Set slave addr. */
> > +     imx_i2c_write_reg((i2c_imx->slave->addr << 1), i2c_imx,
> > + IMX_I2C_IADR);
> > +
> > +     /* Disable i2c module */
> > +     temp = i2c_imx->hwdata->i2cr_ien_opcode
> > +                     ^ I2CR_IEN;
> 
> unnecessary line break.
Ok, no problem. I will remove the line break in v2.
> 
> > +     imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +
> > +     /* Reset status register */
> > +     imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
> > +                       IMX_I2C_I2SR);
> > +
> > +     /* Enable module and enable interrupt from i2c module */
> > +     temp = i2c_imx->hwdata->i2cr_ien_opcode
> > +                     | I2CR_IIEN;
> 
> ditto.
Ok. I will remove the line break in v2.
> 
> > +     imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +
> > +     /* Wait controller to be stable */
> > +     usleep_range(50, 150);
> > +}
> > +
> > +static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx)
> > +{
> > +     unsigned int status, ctl;
> > +     u8 value;
> > +
> > +     if (!i2c_imx->slave) {
> > +             dev_err(&i2c_imx->adapter.dev, "cannot deal with slave
> irq,i2c_imx->slave is null");
> > +             return IRQ_NONE;
> > +     }
> > +
> > +     status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +     ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +     if (status & I2SR_IAL) { /* Arbitration lost */
> > +             i2c_imx_clr_al_bit(i2c_imx);
> > +     } else if (status & I2SR_IAAS) { /* Addressed as a slave */
> > +             if (status & I2SR_SRW) { /* Master wants to read from us*/
> > +                     dev_dbg(&i2c_imx->adapter.dev, "read requested");
> > +                     i2c_slave_event(i2c_imx->slave,
> > + I2C_SLAVE_READ_REQUESTED, &value);
> > +
> > +                     /* Slave transimt */
> 
> s/transimt/transmit/
You are right. I will correct it in v2.
> 
> > +                     ctl |= I2CR_MTX;
> > +                     imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +
> > +                     /* Send data */
> > +                     imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> > +             } else { /* Master wants to write to us */
> > +                     dev_dbg(&i2c_imx->adapter.dev, "write
> requested");
> > +                     i2c_slave_event(i2c_imx->slave,
> > + I2C_SLAVE_WRITE_REQUESTED, &value);
> > +
> > +                     /* Slave receive */
> > +                     ctl &= ~I2CR_MTX;
> > +                     imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +                     /* Dummy read */
> > +                     value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> 
> value is unused, no need to assign a value to it.
Ok, I will correct this in v2.
> 
> > +             }
> > +     } else {
> > +             if (!(ctl & I2CR_MTX)) { /* Receive mode */
> 
> Since you have an 'else' path please convert this to positive logic.
> This makes it easier to read.
Ok,got it, I will convert this to positive logic in v2.
> 
> > +                     if (status & I2SR_IBB) { /* No STOP signal detected
> */
> > +                             ctl &= ~I2CR_MTX;
> > +                             imx_i2c_write_reg(ctl, i2c_imx,
> > + IMX_I2C_I2CR);
> > +
> > +                             value = imx_i2c_read_reg(i2c_imx,
> IMX_I2C_I2DR);
> > +                             i2c_slave_event(i2c_imx->slave,
> I2C_SLAVE_WRITE_RECEIVED, &value);
> > +                     } else { /* STOP signal is detected */
> > +                             dev_dbg(&i2c_imx->adapter.dev,
> > +                                     "STOP signal detected");
> > +                             i2c_slave_event(i2c_imx->slave,
> I2C_SLAVE_STOP, &value);
> > +                     }
> > +             } else { /* Transmit mode */
> > +                     if (!(status & I2SR_RXAK)) {    /* Received ACK */
> 
> Same here.
Ok,got it, I will convert this to positive logic in v2.
> 
> > +                             ctl |= I2CR_MTX;
> > +                             imx_i2c_write_reg(ctl, i2c_imx,
> > + IMX_I2C_I2CR);
> > +
> > +                             i2c_slave_event(i2c_imx->slave,
> > + I2C_SLAVE_READ_PROCESSED, &value);
> > +
> > +                             imx_i2c_write_reg(value, i2c_imx,
> IMX_I2C_I2DR);
> > +                     } else { /* Received NAK */
> > +                             ctl &= ~I2CR_MTX;
> > +                             imx_i2c_write_reg(ctl, i2c_imx,
> IMX_I2C_I2CR);
> > +                             value = imx_i2c_read_reg(i2c_imx,
> > + IMX_I2C_I2DR);
> 
> Also value unused.
Ok,got it, I will correct this in v2.
> 
> > +                     }
> > +             }
> > +     }
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +static int i2c_imx_reg_slave(struct i2c_client *client) {
> > +     struct imx_i2c_struct *i2c_imx =
> > +i2c_get_adapdata(client->adapter);
> > +
> > +     if (i2c_imx->slave)
> > +             return -EINVAL;
> 
> Better -EBUSY?
Yes, you are right, I will correct it in v2.
> 
> Sascha
> 
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=02%7C01%7Cbiwen.li%40nxp.com%7C6741b4f5
> b14646209b6d08d71c0b46cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C637008707067019414&amp;sdata=b95HQ8KRhoLS8R%2BMWyaF
> FTo%2BOr9qqVM45nTn8OoboPg%3D&amp;reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: dts: renesas: r8a77995: draak: Fix backlight regulator name
From: Simon Horman @ 2019-08-09  3:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux-Renesas, Magnus Damm, Geert Uytterhoeven, Linux ARM
In-Reply-To: <CAMuHMdUCb_7=yTb+E-CvbabdRgWRRg6DYzukWqzd_ZCexobnwg@mail.gmail.com>



On 8 August 2019 02:00:20 GMT-07:00, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>Hi Simon,
>
>On Wed, Jul 31, 2019 at 9:37 AM Geert Uytterhoeven
><geert+renesas@glider.be> wrote:
>> Currently there are two nodes named "regulator1" in the Draak DTS: a
>> 3.3V regulator for the eMMC and the LVDS decoder, and a 12V regulator
>> for the backlight.  This causes the former to be overwritten by the
>> latter.
>>
>> Fix this by renaming all regulators with numerical suffixes to use
>named
>> suffixes, which are less likely to conflict.
>>
>> Fixes: 4fbd4158fe8967e9 ("arm64: dts: renesas: r8a77995: draak: Add
>backlight")
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> I guess this is a fix for v5.3?
>
>Do you plan to queue this as a fix for v5.3?
>Thanks!

Likewise, thanks.

Will do.

>
>>
>> This fix takes a slightly different approach than commit
>> 12105cec654cf906 ("arm64: dts: renesas: r8a77990: ebisu: Fix
>backlight
>> regulator numbering"), which just fixed the conflicting numerical
>> suffix.
>> ---
>>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
>b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
>> index 0711170b26b1fe1c..3aa2564dfdc25fff 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
>> @@ -97,7 +97,7 @@
>>                 reg = <0x0 0x48000000 0x0 0x18000000>;
>>         };
>>
>> -       reg_1p8v: regulator0 {
>> +       reg_1p8v: regulator-1p8v {
>>                 compatible = "regulator-fixed";
>>                 regulator-name = "fixed-1.8V";
>>                 regulator-min-microvolt = <1800000>;
>> @@ -106,7 +106,7 @@
>>                 regulator-always-on;
>>         };
>>
>> -       reg_3p3v: regulator1 {
>> +       reg_3p3v: regulator-3p3v {
>>                 compatible = "regulator-fixed";
>>                 regulator-name = "fixed-3.3V";
>>                 regulator-min-microvolt = <3300000>;
>> @@ -115,7 +115,7 @@
>>                 regulator-always-on;
>>         };
>>
>> -       reg_12p0v: regulator1 {
>> +       reg_12p0v: regulator-12p0v {
>>                 compatible = "regulator-fixed";
>>                 regulator-name = "D12.0V";
>>                 regulator-min-microvolt = <12000000>;
>> --
>> 2.17.1
>
>Gr{oetje,eeting}s,
>
>                        Geert

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [EXT] Re: i2c: imx: support slave mode for imx I2C driver
From: Biwen Li @ 2019-08-09  3:18 UTC (permalink / raw)
  To: Wolfram Sang, Joshua Frkuska
  Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
	linux-kernel@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
	Laurentiu Tudor, festevam@gmail.com,
	linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org
In-Reply-To: <20190808200202.GA6609@ninjato>

> > The patch supports slave mode for imx I2C driver
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> 
> Wow, this is much simpler than the other approach flying around:
> 
> http://patchwork.ozlabs.org/patch/1124048/
> 
> Can this one be master and slave on the same bus, too?
At the same time, the same bus is in master mode or slave mode.
> 
> CCing the author of the other patch.
> 
> > ---
> >  drivers/i2c/busses/i2c-imx.c | 199
> > ++++++++++++++++++++++++++++++++---
> >  1 file changed, 185 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx.c
> > b/drivers/i2c/busses/i2c-imx.c index b1b8b938d7f4..f7583a9fa56f 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -202,6 +202,9 @@ struct imx_i2c_struct {
> >  	struct pinctrl_state *pinctrl_pins_gpio;
> >
> >  	struct imx_i2c_dma	*dma;
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +	struct i2c_client		*slave;
> > +#endif /* CONFIG_I2C_SLAVE */
> >  };
> >
> >  static const struct imx_i2c_hwdata imx1_i2c_hwdata = { @@ -583,23
> > +586,40 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
> >  	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);  }
> >
> > -static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
> > +/* Clear interrupt flag bit */
> > +static void i2c_imx_clr_if_bit(struct imx_i2c_struct *i2c_imx)
> >  {
> > -	struct imx_i2c_struct *i2c_imx = dev_id;
> > -	unsigned int temp;
> > +	unsigned int status;
> >
> > -	temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > -	if (temp & I2SR_IIF) {
> > -		/* save status register */
> > -		i2c_imx->i2csr = temp;
> > -		temp &= ~I2SR_IIF;
> > -		temp |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
> > -		imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2SR);
> > -		wake_up(&i2c_imx->queue);
> > -		return IRQ_HANDLED;
> > -	}
> > +	status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +	status &= ~I2SR_IIF;
> > +	status |= (i2c_imx->hwdata->i2sr_clr_opcode & I2SR_IIF);
> > +	imx_i2c_write_reg(status, i2c_imx, IMX_I2C_I2SR); }
> > +
> > +/* Clear arbitration lost bit */
> > +static void i2c_imx_clr_al_bit(struct imx_i2c_struct *i2c_imx) {
> > +	unsigned int status;
> > +
> > +	status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +	status &= ~I2SR_IAL;
> > +	imx_i2c_write_reg(status, i2c_imx, IMX_I2C_I2SR); }
> >
> > -	return IRQ_NONE;
> > +static irqreturn_t i2c_imx_master_isr(struct imx_i2c_struct *i2c_imx)
> > +{
> > +	unsigned int status;
> > +
> > +	dev_dbg(&i2c_imx->adapter.dev, "<%s>: master interrupt\n",
> > +__func__);
> > +
> > +	/* Save status register */
> > +	status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +	i2c_imx->i2csr = status | I2SR_IIF;
> > +
> > +	wake_up(&i2c_imx->queue);
> > +
> > +	return IRQ_HANDLED;
> >  }
> >
> >  static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx, @@
> > -1043,11 +1063,162 @@ static u32 i2c_imx_func(struct i2c_adapter
> *adapter)
> >  		| I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> >  }
> >
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +static void i2c_imx_slave_init(struct imx_i2c_struct *i2c_imx) {
> > +	unsigned int temp;
> > +
> > +	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> > +
> > +	/* Set slave addr. */
> > +	imx_i2c_write_reg((i2c_imx->slave->addr << 1), i2c_imx,
> > +IMX_I2C_IADR);
> > +
> > +	/* Disable i2c module */
> > +	temp = i2c_imx->hwdata->i2cr_ien_opcode
> > +			^ I2CR_IEN;
> > +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +
> > +	/* Reset status register */
> > +	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
> > +			  IMX_I2C_I2SR);
> > +
> > +	/* Enable module and enable interrupt from i2c module */
> > +	temp = i2c_imx->hwdata->i2cr_ien_opcode
> > +			| I2CR_IIEN;
> > +	imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> > +
> > +	/* Wait controller to be stable */
> > +	usleep_range(50, 150);
> > +}
> > +
> > +static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx)
> > +{
> > +	unsigned int status, ctl;
> > +	u8 value;
> > +
> > +	if (!i2c_imx->slave) {
> > +		dev_err(&i2c_imx->adapter.dev, "cannot deal with slave
> irq,i2c_imx->slave is null");
> > +		return IRQ_NONE;
> > +	}
> > +
> > +	status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +	ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +	if (status & I2SR_IAL) { /* Arbitration lost */
> > +		i2c_imx_clr_al_bit(i2c_imx);
> > +	} else if (status & I2SR_IAAS) { /* Addressed as a slave */
> > +		if (status & I2SR_SRW) { /* Master wants to read from us*/
> > +			dev_dbg(&i2c_imx->adapter.dev, "read requested");
> > +			i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_REQUESTED,
> &value);
> > +
> > +			/* Slave transimt */
> > +			ctl |= I2CR_MTX;
> > +			imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +
> > +			/* Send data */
> > +			imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> > +		} else { /* Master wants to write to us */
> > +			dev_dbg(&i2c_imx->adapter.dev, "write requested");
> > +			i2c_slave_event(i2c_imx->slave,
> 	I2C_SLAVE_WRITE_REQUESTED, &value);
> > +
> > +			/* Slave receive */
> > +			ctl &= ~I2CR_MTX;
> > +			imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +			/* Dummy read */
> > +			value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> > +		}
> > +	} else {
> > +		if (!(ctl & I2CR_MTX)) { /* Receive mode */
> > +			if (status & I2SR_IBB) { /* No STOP signal detected */
> > +				ctl &= ~I2CR_MTX;
> > +				imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +
> > +				value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> > +				i2c_slave_event(i2c_imx->slave,
> 	I2C_SLAVE_WRITE_RECEIVED, &value);
> > +			} else { /* STOP signal is detected */
> > +				dev_dbg(&i2c_imx->adapter.dev,
> > +					"STOP signal detected");
> > +				i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &value);
> > +			}
> > +		} else { /* Transmit mode */
> > +			if (!(status & I2SR_RXAK)) {	/* Received ACK */
> > +				ctl |= I2CR_MTX;
> > +				imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +
> > +				i2c_slave_event(i2c_imx->slave,
> 	I2C_SLAVE_READ_PROCESSED, &value);
> > +
> > +				imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> > +			} else { /* Received NAK */
> > +				ctl &= ~I2CR_MTX;
> > +				imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> > +				value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> > +			}
> > +		}
> > +	}
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int i2c_imx_reg_slave(struct i2c_client *client) {
> > +	struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(client->adapter);
> > +
> > +	if (i2c_imx->slave)
> > +		return -EINVAL;
> > +
> > +	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> > +	i2c_imx->slave = client;
> > +
> > +	i2c_imx_slave_init(i2c_imx);
> > +
> > +	return 0;
> > +}
> > +
> > +static int i2c_imx_unreg_slave(struct i2c_client *client) {
> > +	struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(client->adapter);
> > +
> > +	if (!i2c_imx->slave)
> > +		return -EINVAL;
> > +
> > +	i2c_imx->slave = NULL;
> > +
> > +	return 0;
> > +}
> > +#endif /* CONFIG_I2C_SLAVE */
> > +
> >  static const struct i2c_algorithm i2c_imx_algo = {
> >  	.master_xfer	= i2c_imx_xfer,
> >  	.functionality	= i2c_imx_func,
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +	.reg_slave	= i2c_imx_reg_slave,
> > +	.unreg_slave	= i2c_imx_unreg_slave,
> > +#endif /* CONFIG_I2C_SLAVE */
> >  };
> >
> > +static irqreturn_t i2c_imx_isr(int irq, void *dev_id) {
> > +	struct imx_i2c_struct *i2c_imx = dev_id;
> > +	unsigned int status, ctl;
> > +	irqreturn_t irq_status = IRQ_NONE;
> > +
> > +	status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > +	ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +
> > +	if (status & I2SR_IIF) {
> > +		i2c_imx_clr_if_bit(i2c_imx);
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > +		if (ctl & I2CR_MSTA)
> > +			irq_status = i2c_imx_master_isr(i2c_imx);
> > +		else
> > +			irq_status = i2c_imx_slave_isr(i2c_imx); #else
> > +		irq_status = i2c_imx_master_isr(i2c_imx);
> > +
> > +#endif /* CONFIG_I2C_SLAVE */
> > +	}
> > +
> > +	return irq_status;
> > +}
> > +
> >  static int i2c_imx_probe(struct platform_device *pdev)  {
> >  	const struct of_device_id *of_id = of_match_device(i2c_imx_dt_ids,
> > --
> > 2.17.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] ARM: imx: stop adjusting ar8031 phy tx delay
From: André Draszik @ 2019-08-09  3:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kate Stewart, André Draszik, Sascha Hauer, Russell King,
	NXP Linux Team, Pengutronix Kernel Team, Greg Kroah-Hartman,
	Shawn Guo, Thomas Gleixner, Leonard Crestez, Fabio Estevam,
	linux-arm-kernel

Recent changes to the Atheros at803x driver cause
the approach taken here to stop working because
commit 6d4cd041f0af
("net: phy: at803x: disable delay only for RGMII mode")
and commit cd28d1d6e52e
("net: phy: at803x: Disable phy delay for RGMII mode")
fix the AR8031 driver to configure the phy's (RX/TX)
delays as per the 'phy-mode' in the device tree.

In particular, the phy tx (and rx) delays are updated
again as per the 'phy-mode' *after* the code in here
runs.

Things worked before above commits, because the AR8031
comes out of reset with RX delay enabled, and the
at803x driver didn't touch the delay configuration at
all when "rgmii" mode was selected.

It appears the code in here tries to make device
trees work that incorrectly specify "rgmii", but
that can't work any more and it is imperative since
above commits to have the phy-mode configured
correctly in the device tree.

I suspect there are a few imx7d based boards using
the ar8031 phy and phy-mode = "rgmii", but given I
don't know which ones exactly, I am not in a
position to update the respective device trees.

Hence this patch is simply removing the superfluous
code from the imx7d initialisation. An alternative
could be to add a warning instead, but that would
penalize all boards that have been updated already.

Signed-off-by: André Draszik <git@andred.net>
CC: Russell King <linux@armlinux.org.uk>
CC: Shawn Guo <shawnguo@kernel.org>
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Pengutronix Kernel Team <kernel@pengutronix.de>
CC: Fabio Estevam <festevam@gmail.com>
CC: NXP Linux Team <linux-imx@nxp.com>
CC: Kate Stewart <kstewart@linuxfoundation.org>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Leonard Crestez <leonard.crestez@nxp.com>
CC: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-imx/mach-imx7d.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx7d.c b/arch/arm/mach-imx/mach-imx7d.c
index 95713450591a..ebb27592a9f7 100644
--- a/arch/arm/mach-imx/mach-imx7d.c
+++ b/arch/arm/mach-imx/mach-imx7d.c
@@ -30,12 +30,6 @@ static int ar8031_phy_fixup(struct phy_device *dev)
 	val &= ~(0x1 << 8);
 	phy_write(dev, 0xe, val);
 
-	/* introduce tx clock delay */
-	phy_write(dev, 0x1d, 0x5);
-	val = phy_read(dev, 0x1e);
-	val |= 0x0100;
-	phy_write(dev, 0x1e, val);
-
 	return 0;
 }
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] ARM: dts: imx7d: cl-som-imx7: make ethernet work again
From: André Draszik @ 2019-08-09  3:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Rutland, devicetree, Fabio Estevam, linux-arm-kernel,
	André Draszik, Sascha Hauer, Rob Herring, Igor Grinberg,
	Pengutronix Kernel Team, Shawn Guo, Ilya Ledvich, NXP Linux Team

Recent changes to the atheros at803x driver caused
ethernet to stop working on this board.
In particular commit 6d4cd041f0af
("net: phy: at803x: disable delay only for RGMII mode")
and commit cd28d1d6e52e
("net: phy: at803x: Disable phy delay for RGMII mode")
fix the AR8031 driver to configure the phy's (RX/TX)
delays as per the 'phy-mode' in the device tree.

This now prevents ethernet from working on this board.

It used to work before those commits, because the
AR8031 comes out of reset with RX delay enabled, and
the at803x driver didn't touch the delay configuration
at all when "rgmii" mode was selected, and because
arch/arm/mach-imx/mach-imx7d.c:ar8031_phy_fixup()
unconditionally enables TX delay.

Since above commits ar8031_phy_fixup() also has no
effect anymore, and the end-result is that all delays
are disabled in the phy, no ethernet.

Update the device tree to restore functionality.

Signed-off-by: André Draszik <git@andred.net>
CC: Ilya Ledvich <ilya@compulab.co.il>
CC: Igor Grinberg <grinberg@compulab.co.il>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Shawn Guo <shawnguo@kernel.org>
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Pengutronix Kernel Team <kernel@pengutronix.de>
CC: Fabio Estevam <festevam@gmail.com>
CC: NXP Linux Team <linux-imx@nxp.com>
CC: devicetree@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
---
 arch/arm/boot/dts/imx7d-cl-som-imx7.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
index e61567437d73..62d5e9a4a781 100644
--- a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
+++ b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
@@ -44,7 +44,7 @@
 			  <&clks IMX7D_ENET1_TIME_ROOT_CLK>;
 	assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>;
 	assigned-clock-rates = <0>, <100000000>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy0>;
 	fsl,magic-packet;
 	status = "okay";
@@ -70,7 +70,7 @@
 			  <&clks IMX7D_ENET2_TIME_ROOT_CLK>;
 	assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>;
 	assigned-clock-rates = <0>, <100000000>;
-	phy-mode = "rgmii";
+	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy1>;
 	fsl,magic-packet;
 	status = "okay";
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 1/2 v2] tracing/arm64: Have max stack tracer handle the case of return address after data
From: Steven Rostedt @ 2019-08-09  2:24 UTC (permalink / raw)
  To: Jiping Ma
  Cc: catalin.marinas, will.deacon, linux-kernel, mingo, Joel Fernandes,
	Will Deacon, linux-arm-kernel
In-Reply-To: <21530ce5-3847-c669-2a64-7c59ffb45f35@windriver.com>

On Fri, 9 Aug 2019 10:17:19 +0800
Jiping Ma <Jiping.Ma2@windriver.com> wrote:

> On 2019年08月09日 01:24, Steven Rostedt wrote:
> > On Thu, 8 Aug 2019 18:11:53 +0100
> > Will Deacon <will@kernel.org> wrote:
> >  
> >>> We could make it more descriptive of what it will do and not the reason
> >>> for why it is done...
> >>>
> >>>
> >>>    ARCH_FTRACE_SHIFT_STACK_TRACER  
> >> Acked-by: Will Deacon <will@kernel.org>  
> > Thanks Will!
> >
> > Here's the official patch.
> >
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > Most archs (well at least x86) store the function call return address on the
> > stack before storing the local variables for the function. The max stack
> > tracer depends on this in its algorithm to display the stack size of each
> > function it finds in the back trace.
> >
> > Some archs (arm64), may store the return address (from its link register)
> > just before calling a nested function. There's no reason to save the link
> > register on leaf functions, as it wont be updated. This breaks the algorithm
> > of the max stack tracer.
> >
> > Add a new define ARCH_RET_ADDR_AFTER_LOCAL_VARS that an architecture may set  
> 
> ARCH_FTRACE_SHIFT_STACK_TRACER is used in the code.

Ah, I did a s/x/y/ to the diff of the patch, but not the change log.
Thanks for pointing that out. I also need to update the comment in 2/2.

-- Steve

> 
> Jiping
> 
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Chanwoo Choi @ 2019-08-09  2:17 UTC (permalink / raw)
  To: Artur Świgoń, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-pm, dri-devel
  Cc: sw0312.kim, krzk, inki.dae, myungjoo.ham, georgi.djakov,
	m.szyprowski
In-Reply-To: <7171e3c56b0dca8a81058e77dd76bd1e684e7778.camel@partner.samsung.com>

Hi,

We need to discuss how to change or do refactoring on v2.
Actually, I don't know the your opinion and how to do it on v2.
You have to reply the answer and then after finished the discussion,
I recommend that you would rework and resend the v2 patches.

On 19. 8. 8. 오후 10:18, Artur Świgoń wrote:
> Hi,
> 
> Thank you for your remarks. I will take them into account while preparing RFCv2.
> 
> On Mon, 2019-07-29 at 10:52 +0900, Chanwoo Choi wrote:
>> Hi,
>>
>> On 19. 7. 23. 오후 9:20, Artur Świgoń wrote:
>>> This patch adds interconnect functionality to the exynos-bus devfreq
>>> driver.
>>>
>>> The SoC topology is a graph (or, more specifically, a tree) and most of its
>>> edges are taken from the devfreq parent-child hierarchy (cf.
>>> Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
>>> patch adds missing edges to the DT (under the name 'parent'). Due to
>>> unspecified relative probing order, -EPROBE_DEFER may be propagated to
>>> guarantee that a child is probed before its parent.
>>>
>>> Each bus is now an interconnect provider and an interconnect node as well
>>> (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers
>>> itself as a node. Node IDs are not hardcoded but rather assigned at
>>> runtime, in probing order (subject to the above-mentioned exception
>>> regarding relative order). This approach allows for using this driver with
>>> various Exynos SoCs.
>>>
>>> The devfreq target() callback provided by exynos-bus now selects either the
>>> frequency calculated by the devfreq governor or the frequency requested via
>>> the interconnect API for the given node, whichever is higher.
>>
>> Basically, I agree to support the QoS requirement between devices.
>> But, I think that need to consider the multiple cases.
>>
>>
>> 1. When changing the devfreq governor by user,
>> For example of the connection between bus_dmc/leftbus/display on patch8,
>> there are possible multiple cases with various devfreq governor
>> which is changed on the runtime by user through sysfs interface.
>>
>> If users changes the devfreq governor as following:
>> Before,
>> - bus_dmc (simple_ondemand, available frequency 100/200/300/400 MHz)
>> --> bus_leftbus(simple_ondemand, available frequency 100/200/300/400 MHz)
>> ----> bus_display(passive)
>>
>> After changed governor of bus_dmc,
>> if the min_freq by interconnect requirement is 400Mhz,
>> - bus_dmc (powersave) : min_freq and max_freq and cur_freq is 100MHz
>> --> bus_leftbus(simple_ondemand) : cur_freq is 400Mhz
>> ----> bus_display(passive)
>>
>> The final frequency is 400MHz of bus_dmc
>> even if the min_freq/max_freq/cur_freq is 100MHz.
>> It cannot show the correct min_freq/max_freq through
>> devfreq sysfs interface.
>>
>>
>> 2. When disabling the some frequency by devfreq-thermal throttling,
>> This patch checks the min_freq of interconnect requirement
>> in the exynos_bus_target() and exynos_bus_passive_target().
>> Also, it cannot show the correct min_freq/max_freq through
>> devfreq sysfs interface.
>>
>> For example of bus_dmc bus,
>> - The available frequencies are 100MHz, 200MHz, 300MHz, 400MHz
>> - Disable 400MHz by devfreq-thermal throttling 
>> - min_freq is 100MHz
>> - max_freq is 300MHz
>> - min_freq of interconnect is 400MHz
>>
>> In result, the final frequency is 400MHz by exynos_bus_target()
>> There are no problem for working. But, the user cannot know
>> reason why cur_freq is 400MHz even if max_freq is 300MHz.
>>
>> Basically, update_devfreq() considers the all constraints
>> of min_freq/max_freq to decide the proper target frequency.
>>
>>
>> 3.
>> I think that the exynos_bus_passive_target() is used for devfreq device
>> using 'passive' governor. The frequency already depends on the parent device.
>>
>> If already the parent devfreq device like bus_leftbus consider
>> the minimum frequency of QoS requirement like interconnect,
>> it is not necessary. The next frequency of devfreq device
>> with 'passive' governor, it will apply the QoS requirement
>> without any additional code.
>>
>>>
>>> Please note that it is not an error when CONFIG_INTERCONNECT is 'n', in
>>> which case all interconnect API functions are no-op.
>>>
>>> Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
>>> ---
>>>  drivers/devfreq/exynos-bus.c | 145 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 145 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>> index 412511ca7703..12fb7c84ae50 100644
>>> --- a/drivers/devfreq/exynos-bus.c
>>> +++ b/drivers/devfreq/exynos-bus.c
>>> @@ -14,6 +14,7 @@
>>>  #include <linux/devfreq-event.h>
>>>  #include <linux/device.h>
>>>  #include <linux/export.h>
>>> +#include <linux/interconnect-provider.h>
>>>  #include <linux/module.h>
>>>  #include <linux/of.h>
>>>  #include <linux/pm_opp.h>
>>> @@ -23,6 +24,8 @@
>>>  #define DEFAULT_SATURATION_RATIO	40
>>>  #define DEFAULT_VOLTAGE_TOLERANCE	2
>>>  
>>> +#define icc_units_to_hz(x) ((x) * 1000UL / 8)
>>> +
>>>  struct exynos_bus {
>>>  	struct device *dev;
>>>  
>>> @@ -31,12 +34,17 @@ struct exynos_bus {
>>>  	unsigned int edev_count;
>>>  	struct mutex lock;
>>>  
>>> +	unsigned long min_freq;
>>>  	unsigned long curr_freq;
>>>  
>>>  	struct regulator *regulator;
>>>  	struct clk *clk;
>>>  	unsigned int voltage_tolerance;
>>>  	unsigned int ratio;
>>> +
>>> +	/* One provider per bus, one node per provider */
>>> +	struct icc_provider provider;
>>> +	struct icc_node *node;
>>>  };
>>>  
>>>  /*
>>> @@ -61,6 +69,13 @@ exynos_bus_ops_edev(enable_edev);
>>>  exynos_bus_ops_edev(disable_edev);
>>>  exynos_bus_ops_edev(set_event);
>>>  
>>> +static int exynos_bus_next_id(void)
>>> +{
>>> +	static int exynos_bus_node_id;
>>> +
>>> +	return exynos_bus_node_id++;
>>> +}
>>> +
>>>  static int exynos_bus_get_event(struct exynos_bus *bus,
>>>  				struct devfreq_event_data *edata)
>>>  {
>>> @@ -98,6 +113,8 @@ static int exynos_bus_target(struct device *dev, unsigned
>>> long *freq, u32 flags)
>>>  	unsigned long old_freq, new_freq, new_volt, tol;
>>>  	int ret = 0;
>>>  
>>> +	*freq = max(*freq, bus->min_freq);
>>> +
>>>  	/* Get new opp-bus instance according to new bus clock */
>>>  	new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>  	if (IS_ERR(new_opp)) {
>>> @@ -208,6 +225,8 @@ static int exynos_bus_passive_target(struct device *dev,
>>> unsigned long *freq,
>>>  	unsigned long old_freq, new_freq;
>>>  	int ret = 0;
>>>  
>>> +	*freq = max(*freq, bus->min_freq);
>>> +
>>>  	/* Get new opp-bus instance according to new bus clock */
>>>  	new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>  	if (IS_ERR(new_opp)) {
>>> @@ -251,6 +270,35 @@ static void exynos_bus_passive_exit(struct device *dev)
>>>  	clk_disable_unprepare(bus->clk);
>>>  }
>>>  
>>> +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
>>> +{
>>> +	struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
>>> +
>>> +	src_bus->min_freq = icc_units_to_hz(src->peak_bw);
>>> +	dst_bus->min_freq = icc_units_to_hz(dst->peak_bw);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
>>> +				    u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
>>> +{
>>> +	*agg_peak = *agg_avg = peak_bw;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
>>> +					     void *data)
>>> +{
>>> +	struct exynos_bus *bus = data;
>>> +
>>> +	if (spec->np != bus->dev->of_node)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	return bus->node;
>>> +}
>>> +
>>>  static int exynos_bus_parent_parse_of(struct device_node *np,
>>>  					struct exynos_bus *bus)
>>>  {
>>> @@ -469,6 +517,95 @@ static int exynos_bus_profile_init_passive(struct
>>> exynos_bus *bus,
>>>  	return ret;
>>>  }
>>>  
>>> +static int exynos_bus_icc_connect(struct exynos_bus *bus)
>>> +{
>>> +	struct device_node *np = bus->dev->of_node;
>>> +	struct devfreq *parent_devfreq;
>>> +	struct icc_node *parent_node = NULL;
>>> +	struct of_phandle_args args;
>>> +	int ret = 0;
>>> +
>>> +	parent_devfreq = devfreq_get_devfreq_by_phandle(bus->dev, 0);
>>> +	if (!IS_ERR(parent_devfreq)) {
>>> +		struct exynos_bus *parent_bus;
>>> +
>>> +		parent_bus = dev_get_drvdata(parent_devfreq->dev.parent);
>>> +		parent_node = parent_bus->node;
>>> +	} else {
>>> +		/* Look for parent in DT */
>>> +		int num = of_count_phandle_with_args(np, "parent",
>>> +						     "#interconnect-cells");
>>> +		if (num != 1)
>>> +			goto out;
>>> +
>>> +		ret = of_parse_phandle_with_args(np, "parent",
>>> +						 "#interconnect-cells",
>>> +						 0, &args);
>>> +		if (ret < 0)
>>> +			goto out;
>>> +
>>> +		of_node_put(args.np);
>>> +
>>> +		parent_node = of_icc_get_from_provider(&args);
>>> +		if (IS_ERR(parent_node)) {
>>> +			/* May be -EPROBE_DEFER */
>>> +			ret = PTR_ERR(parent_node);
>>> +			goto out;
>>> +		}
>>> +	}
>>> +
>>> +	ret = icc_link_create(bus->node, parent_node->id);
>>> +
>>> +out:
>>> +	return ret;
>>> +}
>>> +
>>> +static int exynos_bus_icc_init(struct exynos_bus *bus)
>>> +{
>>> +	struct device *dev = bus->dev;
>>> +	struct icc_provider *provider = &bus->provider;
>>> +	struct icc_node *node;
>>> +	int id, ret;
>>> +
>>> +	/* Initialize the interconnect provider */
>>> +	provider->set = exynos_bus_icc_set;
>>> +	provider->aggregate = exynos_bus_icc_aggregate;
>>> +	provider->xlate = exynos_bus_icc_xlate;
>>> +	provider->dev = dev;
>>> +	provider->data = bus;
>>> +
>>> +	ret = icc_provider_add(provider);
>>> +	if (ret < 0)
>>> +		goto out;
>>> +
>>> +	id = exynos_bus_next_id();
>>> +	node = icc_node_create(id);
>>> +	if (IS_ERR(node)) {
>>> +		ret = PTR_ERR(node);
>>> +		goto err_node;
>>> +	}
>>> +
>>> +	bus->node = node;
>>> +	node->name = dev->of_node->name;
>>> +	node->data = bus;
>>> +	icc_node_add(node, provider);
>>> +
>>> +	ret = exynos_bus_icc_connect(bus);
>>> +	if (ret < 0)
>>> +		goto err_connect;
>>> +
>>> +out:
>>> +	return ret;
>>> +
>>> +err_connect:
>>> +	icc_node_del(node);
>>> +	icc_node_destroy(id);
>>> +err_node:
>>> +	icc_provider_del(provider);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>>  static int exynos_bus_probe(struct platform_device *pdev)
>>>  {
>>>  	struct device *dev = &pdev->dev;
>>> @@ -517,6 +654,14 @@ static int exynos_bus_probe(struct platform_device
>>> *pdev)
>>>  			goto err;
>>>  	}
>>>  
>>> +	/*
>>> +	 * Initialize interconnect provider. A return value of -ENOTSUPP means
>>> +	 * that CONFIG_INTERCONNECT is disabled.
>>> +	 */
>>> +	ret = exynos_bus_icc_init(bus);
>>> +	if (ret < 0 && ret != -ENOTSUPP)
>>> +		goto err;
>>> +
>>>  	max_state = bus->devfreq->profile->max_state;
>>>  	min_freq = (bus->devfreq->profile->freq_table[0] / 1000);
>>>  	max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000);
>>>
>>
>>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 38/41] powerpc: convert put_page() to put_user_page*()
From: John Hubbard @ 2019-08-09  1:26 UTC (permalink / raw)
  To: Michael Ellerman, Andrew Morton
  Cc: linux-fbdev, Jan Kara, kvm, Benjamin Herrenschmidt, Dave Hansen,
	Dave Chinner, dri-devel, linux-mm, sparclinux, Ira Weiny,
	ceph-devel, devel, rds-devel, linux-rdma, x86, amd-gfx,
	Christoph Hellwig, Christoph Hellwig, Jason Gunthorpe, xen-devel,
	devel, linux-media, intel-gfx, linux-block,
	Jérôme Glisse, linux-rpi-kernel, Dan Williams,
	linux-arm-kernel, linux-nfs, netdev, LKML, linux-xfs,
	linux-crypto, linux-fsdevel, linuxppc-dev
In-Reply-To: <87k1botdpx.fsf@concordia.ellerman.id.au>

On 8/7/19 10:42 PM, Michael Ellerman wrote:
> Hi John,
> 
> john.hubbard@gmail.com writes:
>> diff --git a/arch/powerpc/mm/book3s64/iommu_api.c b/arch/powerpc/mm/book3s64/iommu_api.c
>> index b056cae3388b..e126193ba295 100644
>> --- a/arch/powerpc/mm/book3s64/iommu_api.c
>> +++ b/arch/powerpc/mm/book3s64/iommu_api.c
>> @@ -203,6 +202,7 @@ static void mm_iommu_unpin(struct mm_iommu_table_group_mem_t *mem)
>>  {
>>  	long i;
>>  	struct page *page = NULL;
>> +	bool dirty = false;
> 
> I don't think you need that initialisation do you?
> 

Nope, it can go. Fixed locally, thanks.

Did you get a chance to look at enough of the other bits to feel comfortable 
with the patch, overall?

thanks,
-- 
John Hubbard
NVIDIA

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] mtd: spi-nor: aspeed-smc: Add of_node_put()
From: Andrew Jeffery @ 2019-08-09  0:38 UTC (permalink / raw)
  To: Nishka Dasgupta, marek.vasut, tudor.ambarus, dwmw2,
	computersforpeace, miquel.raynal, richard, vigneshr, Joel Stanley,
	linux-mtd, linux-arm-kernel, linux-aspeed
In-Reply-To: <20190808075104.15928-1-nishkadg.linux@gmail.com>



On Thu, 8 Aug 2019, at 17:21, Nishka Dasgupta wrote:
> Each iteration of for_each_available_child_of_node puts the previous
> node, but in the case of a break from the middle of the loop, there is
> no put, thus causing a memory leak. Upon termination of the loop
> (whether by break or a natural exit), either ret will have a non-zero
> value or child will be NULL. Hence add an of_node_put() that will
> execute only when ret has a non-zero value, as calling of_node_put() on
> a possible NULL value does not cause any further issues.
> Issue found with Coccinelle.
> 
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 0/6] arm64: g12a: add support for DVFS
From: Kevin Hilman @ 2019-08-08 23:56 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong
In-Reply-To: <7hwofrh1md.fsf@baylibre.com>

Kevin Hilman <khilman@baylibre.com> writes:

> Neil Armstrong <narmstrong@baylibre.com> writes:
>
>> The G12A & G12B SoCs has kernel controllable CPU clocks and PWMs for
>> voltage regulators.
>>
>> This patchsets moves the meson-g12a.dtsi to meson-g12-common.dtsi to simplify
>> handling the G12A & G12B differences in the meson-g12a.dtsi & meson-g12b.dtsi
>> files, like the OPPs and CPU nodes.
>>
>> Then G12A & G12B OPP tables are added, followed by the CPU voltages regulators
>> in each boards DT.
>>
>> It was voluntary chosen to enabled DVFS (CPU regulator and CPU clocks) only
>> in boards, to make sure only tested boards has DVFS enabled.
>>
>> This patchset :
>> - moves the G12A DT to a common g12a-common dtsi
>> - adds the G12A and G12B OPPs
>> - enables DVFS on all supported boards
>>
>> Dependencies:
>> - None
>
> Not quite.  The last patch to enable DVFS on odroid-n2 has a build-time
> dependency on the clock series that adds the CPUB clock.
>
> I'll apply the rest of the series to v5.4/dt64 until there's a stable
> clock tag I can use for the clocks.

In order to test this, I noticed another dependency needed for the PWM
regulators to work:

   https://lore.kernel.org/linux-amlogic/20190729125838.6498-1-narmstrong@baylibre.com/

With that and the clock deps, it's working well on my odroid-n2.

Tested-by: Kevin Hilman <khilman@baylibre.com>

Thanks,

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 13/22] input: omap: void using mach/*.h headers
From: Sebastian Reichel @ 2019-08-08 23:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Arnd Bergmann, Bartlomiej Zolnierkiewicz, Tony Lindgren,
	Greg Kroah-Hartman, Linus Walleij, Aaro Koskinen,
	Linux Kernel Mailing List, Tomi Valkeinen,
	open list:HID CORE LAYER, linux-omap, Linux ARM
In-Reply-To: <20190808221950.GG178933@dtor-ws>


[-- Attachment #1.1: Type: text/plain, Size: 1631 bytes --]

Hi,

On Thu, Aug 08, 2019 at 03:19:50PM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 08, 2019 at 11:46:45PM +0200, Arnd Bergmann wrote:
> > On Thu, Aug 8, 2019 at 11:43 PM Dmitry Torokhov wrote:
> > > On Thu, Aug 08, 2019 at 11:22:22PM +0200, Arnd Bergmann wrote:
> > > > By using the new linux/soc/ti/omap1-io.h header instead,
> > > > compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
> > > > conversion of omap1 may eventually be possible.
> > > >
> > > > The warning in the header file gets removed in order to
> > > > allow CONFIG_COMPILE_TEST.
> > >
> > > Given that we want to migrate people off this driver everywhere but
> > > OMAP1 I wonder why we would want to improve compile coverage of it.
> > 
> > Mainly for consistency: I'm converting all omap1 drivers in this series to
> > not rely on mach/* headers and to let them be compiled standalone.
> > The other drivers don't have a replacement, so I could treat this different
> > from the rest and skip the Kconfig and platform_data changes if you
> > prefer.
> 
> Yes, because at least with the version you posted we are losing the
> #warning telling people to move to matrix_keypad. We could do:
> 
> #ifndef CONFIG_COMPILE_TEST
> #warning ...
> #endif
> 
> if you really want to allow compiling standalone for testing.

FWIW the driver depends on ARCH_OMAP1 and the warning is
only printed for !ARCH_OMAP1. In other words: The warning
is never printed at the moment. All OMAP2+ boards moved to
matrix-keypad long time ago and the driver does not support
OMAP2+ anymore since f799a3d8fe170 from 2012.

-- Sebastian

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [linux-sunxi] [PATCH 0/3] Add basic support for RTC on Allwinner H6 SoC
From: Alexandre Belloni @ 2019-08-08 23:39 UTC (permalink / raw)
  To: Chen-Yu Tsai, Mark Rutland, Alessandro Zummo, devicetree,
	Maxime Ripard, linux-kernel, linux-sunxi, Rob Herring,
	linux-arm-kernel, linux-rtc
In-Reply-To: <20190808121237.g6twq2nh3sayu3vx@core.my.home>

On 08/08/2019 14:12:37+0200, Ondřej Jirman wrote:
> On Wed, Aug 07, 2019 at 12:55:02PM +0200, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 06/08/2019 20:30:45+0200, Ondřej Jirman wrote:
> > > Maybe whether XO or DCXO is used also matters if you want to do some fine
> > > tunning of DCXO (control register has pletny of options), but that's probably
> > > better done in u-boot. And there's still no need to read HOSC source from DT.
> > > The driver can just check compatible, and if it is H6 and OSC_CLK_SRC_SEL is 1,
> > > it can do it's DCXO tunning, or whatever. But neither OS nor bootloader will
> > > be using this info to gate/disable the osciallator.
> > > 
> > 
> > It is actually useful to be able to tweak the crystal tuning at
> > runtime to be able to reduce clock drift and compare with a reliable
> > source (e.g. NTP).
> 
> I don't think there's a Linux kernel API that you can use to achieve that, so
> that's a rather theoretical concern at the moment.
> 

There is /sys/class/rtc/rtcX/offset which is even properly documented.

The reason I asked is that some RTCs have both analog (changing the
oscillator capacitance) and digital (changing the RTC counter) so I'm
wondering whether this interface should be extended.

> Also there are multiple clocks, that can drive the RTC, and you usually don't
> drive it from 24MHz DCXO oscillator. The reason is that you'd have to deal with
> the fact that the clock for RTC then becomes 24000000/750 (750 is fixed
> divider), which is 32000.
> 
> So if you want to get 32768Hz for RTC by tuning the DCXO, it would have to have
> 24 576 000 Hz. And even if you could achieve that (doubtful), it would throw off
> timings in the rest of the system (say UART, USB, CPU, display ctl) in a major way.
> 
> I guess you can try tuning 24MHz oscillator so that it's closer to the
> real-world 24MHz via NTP reference for other reasons. But it would be
> complicated, and require precise interaction with other components, like using
> HW timers sourced from 24MHz HOSC clock, because you can't use CPU's timers,
> because of inaccuracies introduced during DVFS, for example.
> 
> regards,
> 	o.
> 
> > I'm curious, what kind of options does this RTC have?
> > 
> > -- 
> > Alexandre Belloni, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v19 00/15] arm64: untag user pointers passed to the kernel
From: Kees Cook @ 2019-08-08 23:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mark Rutland, kvm, Christian Koenig, Szabolcs Nagy,
	Catalin Marinas, Will Deacon, dri-devel, Kostya Serebryany,
	Khalid Aziz, Lee Smith, open list:KERNEL SELFTEST FRAMEWORK,
	Vincenzo Frascino, Will Deacon, Jacob Bramley, Leon Romanovsky,
	linux-rdma, amd-gfx, Christoph Hellwig, Jason Gunthorpe,
	Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, linux-media,
	Ruben Ayrapetyan, Andrey Konovalov, Kevin Brodsky,
	Alex Williamson, Mauro Carvalho Chehab, Linux ARM,
	Linux Memory Management List, Greg Kroah-Hartman, Felix Kuehling,
	LKML, Jens Wiklander, Ramana Radhakrishnan, Alexander Deucher,
	enh, Robin Murphy, Yishai Hadas, Luc Van Oostenryck
In-Reply-To: <20190808153300.09d3eb80772515f0ea062833@linux-foundation.org>

On Thu, Aug 08, 2019 at 03:33:00PM -0700, Andrew Morton wrote:
> On Thu, 8 Aug 2019 14:12:19 -0700 Kees Cook <keescook@chromium.org> wrote:
> 
> > > The ones that are left are the mm ones: 4, 5, 6, 7 and 8.
> > > 
> > > Andrew, could you take a look and give your Acked-by or pick them up directly?
> > 
> > Given the subsystem Acks, it seems like 3-10 and 12 could all just go
> > via Andrew? I hope he agrees. :)
> 
> I'll grab everything that has not yet appeared in linux-next.  If more
> of these patches appear in linux-next I'll drop those as well.
> 
> The review discussion against " [PATCH v19 02/15] arm64: Introduce
> prctl() options to control the tagged user addresses ABI" has petered
> out inconclusively.  prctl() vs arch_prctl().

I've always disliked arch_prctl() existing at all. Given that tagging is
likely to be a multi-architectural feature, it seems like the controls
should live in prctl() to me.

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v19 00/15] arm64: untag user pointers passed to the kernel
From: Andrew Morton @ 2019-08-08 22:33 UTC (permalink / raw)
  To: Kees Cook
  Cc: Mark Rutland, kvm, Christian Koenig, Szabolcs Nagy,
	Catalin Marinas, Will Deacon, dri-devel, Kostya Serebryany,
	Khalid Aziz, Lee Smith, open list:KERNEL SELFTEST FRAMEWORK,
	Vincenzo Frascino, Will Deacon, Jacob Bramley, Leon Romanovsky,
	linux-rdma, amd-gfx, Christoph Hellwig, Jason Gunthorpe,
	Dmitry Vyukov, Dave Martin, Evgeniy Stepanov, linux-media,
	Ruben Ayrapetyan, Andrey Konovalov, Kevin Brodsky,
	Alex Williamson, Mauro Carvalho Chehab, Linux ARM,
	Linux Memory Management List, Greg Kroah-Hartman, Felix Kuehling,
	LKML, Jens Wiklander, Ramana Radhakrishnan, Alexander Deucher,
	enh, Robin Murphy, Yishai Hadas, Luc Van Oostenryck
In-Reply-To: <201908081410.C16D2BD@keescook>

On Thu, 8 Aug 2019 14:12:19 -0700 Kees Cook <keescook@chromium.org> wrote:

> > The ones that are left are the mm ones: 4, 5, 6, 7 and 8.
> > 
> > Andrew, could you take a look and give your Acked-by or pick them up directly?
> 
> Given the subsystem Acks, it seems like 3-10 and 12 could all just go
> via Andrew? I hope he agrees. :)

I'll grab everything that has not yet appeared in linux-next.  If more
of these patches appear in linux-next I'll drop those as well.

The review discussion against " [PATCH v19 02/15] arm64: Introduce
prctl() options to control the tagged user addresses ABI" has petered
out inconclusively.  prctl() vs arch_prctl().


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [Tee-dev] [RFC v2 2/6] tee: enable support to register kernel memory
From: Stuart Yoder @ 2019-08-08 22:26 UTC (permalink / raw)
  To: Sumit Garg, keyrings, linux-integrity, linux-security-module
  Cc: corbet, daniel.thompson, ard.biesheuvel, jejb, linux-doc,
	linux-kernel, zohar, tee-dev, jarkko.sakkinen, dhowells, casey,
	linux-arm-kernel, serge
In-Reply-To: <1564489420-677-3-git-send-email-sumit.garg@linaro.org>



On 7/30/19 7:23 AM, Sumit Garg wrote:

> @@ -264,7 +266,17 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
>   		goto err;
>   	}
>   
> -	rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages);
> +	if (flags & TEE_SHM_USER_MAPPED) {
> +		rc = get_user_pages_fast(start, num_pages, FOLL_WRITE,
> +					 shm->pages);
> +	} else {
> +		const struct kvec kiov = {
> +			.iov_base = (void *)start,
> +			.iov_len = PAGE_SIZE
> +		};
> +
> +		rc = get_kernel_pages(&kiov, num_pages, 0, shm->pages);

Passing a single kvec struct is temporary I assume?  Because as currently
written this will only work with num_pages==1.

Stuart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 1/2] ACPI/PPTT: Add support for ACPI 6.3 thread flag
From: Robert Richter @ 2019-08-08 22:25 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: lorenzo.pieralisi@arm.com, rric@kernel.org,
	catalin.marinas@arm.com, rjw@rjwysocki.net,
	linux-acpi@vger.kernel.org, sudeep.holla@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org, lenb@kernel.org
In-Reply-To: <20190808204007.30110-2-jeremy.linton@arm.com>

On 08.08.19 15:40:06, Jeremy Linton wrote:
> ACPI 6.3 adds a flag to the CPU node to indicate whether
> the given PE is a thread. Add a function to return that
> information for a given linux logical CPU.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/acpi/pptt.c  | 53 +++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/acpi.h |  5 +++++
>  2 files changed, 57 insertions(+), 1 deletion(-)

Reviewed-by: Robert Richter <rrichter@marvell.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 07/22] ARM: omap1: move perseus spi pinconf to board file
From: Mark Brown @ 2019-08-08 22:24 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Aaro Koskinen, Tony Lindgren, Greg Kroah-Hartman, Linus Walleij,
	Bartlomiej Zolnierkiewicz, linux-kernel, linux-spi,
	Tomi Valkeinen, linux-omap, linux-arm-kernel, Boris Brezillon
In-Reply-To: <20190808212234.2213262-8-arnd@arndb.de>


[-- Attachment #1.1: Type: text/plain, Size: 339 bytes --]

On Thu, Aug 08, 2019 at 11:22:16PM +0200, Arnd Bergmann wrote:
> The driver has always had a FIXME about this, and it seems
> like this trivial code move avoids a mach header inclusion,
> so just do it.

This appears to be part of a series but I've no cover letter or anything
else from it.  What's the story for dependencies and merging?

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 2/2] arm64: topology: Use PPTT to determine if PE is a thread
From: Robert Richter @ 2019-08-08 22:23 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: lorenzo.pieralisi@arm.com, rric@kernel.org,
	catalin.marinas@arm.com, rjw@rjwysocki.net,
	linux-acpi@vger.kernel.org, sudeep.holla@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org, lenb@kernel.org
In-Reply-To: <20190808204007.30110-3-jeremy.linton@arm.com>

On 08.08.19 15:40:07, Jeremy Linton wrote:
> ACPI 6.3 adds a thread flag to represent if a CPU/PE is
> actually a thread. Given that the MPIDR_MT bit may not
> represent this information consistently on homogeneous machines
> we should prefer the PPTT flag if its available.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/kernel/topology.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 0825c4a856e3..bad9c42ea825 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -340,17 +340,28 @@ void remove_cpu_topology(unsigned int cpu)
>  }
>  
>  #ifdef CONFIG_ACPI
> +static int __init acpi_cpu_is_threaded(int cpu)
> +{
> +	int is_threaded = acpi_pptt_cpu_is_thread(cpu);
> +
> +	/*
> +	 * if the PPTT doesn't have thread information, assume a homogeneous
> +	 * machine and return the current CPU's thread state.
> +	 */
> +	if (is_threaded < 0)
> +		is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;

Since this is no longer bool now, better have a !!(...) or (...) != 0
here.

> +
> +	return is_threaded;
> +}
> +
>  /*
>   * Propagate the topology information of the processor_topology_node tree to the
>   * cpu_topology array.
>   */
>  static int __init parse_acpi_topology(void)
>  {
> -	bool is_threaded;
>  	int cpu, topology_id;
>  
> -	is_threaded = read_cpuid_mpidr() & MPIDR_MT_BITMASK;
> -
>  	for_each_possible_cpu(cpu) {
>  		int i, cache_id;
>  
> @@ -358,7 +369,7 @@ static int __init parse_acpi_topology(void)
>  		if (topology_id < 0)
>  			return topology_id;
>  
> -		if (is_threaded) {
> +		if (acpi_cpu_is_threaded(cpu)) {
>  			cpu_topology[cpu].thread_id = topology_id;
>  			topology_id = find_acpi_cpu_topology(cpu, 1);
>  			cpu_topology[cpu].core_id   = topology_id;

Looks otherwise ok.

Reviewed-by: Robert Richter <rrichter@marvell.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 13/22] input: omap: void using mach/*.h headers
From: Dmitry Torokhov @ 2019-08-08 22:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Aaro Koskinen, Tony Lindgren, Greg Kroah-Hartman, Linus Walleij,
	Bartlomiej Zolnierkiewicz, Linux Kernel Mailing List,
	Tomi Valkeinen, open list:HID CORE LAYER, linux-omap, Linux ARM
In-Reply-To: <CAK8P3a2TOcjxwCBGkZAhMAf9HuTL=FAB1e0=FAg+oHB0U1nJ0A@mail.gmail.com>

On Thu, Aug 08, 2019 at 11:46:45PM +0200, Arnd Bergmann wrote:
> On Thu, Aug 8, 2019 at 11:43 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Arnd,
> >
> > On Thu, Aug 08, 2019 at 11:22:22PM +0200, Arnd Bergmann wrote:
> > > By using the new linux/soc/ti/omap1-io.h header instead,
> > > compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
> > > conversion of omap1 may eventually be possible.
> > >
> > > The warning in the header file gets removed in order to
> > > allow CONFIG_COMPILE_TEST.
> >
> > Given that we want to migrate people off this driver everywhere but
> > OMAP1 I wonder why we would want to improve compile coverage of it.
> 
> Mainly for consistency: I'm converting all omap1 drivers in this series to
> not rely on mach/* headers and to let them be compiled standalone.
> The other drivers don't have a replacement, so I could treat this different
> from the rest and skip the Kconfig and platform_data changes if you
> prefer.

Yes, because at least with the version you posted we are losing the
#warning telling people to move to matrix_keypad. We could do:

#ifndef CONFIG_COMPILE_TEST
#warning ...
#endif

if you really want to allow compiling standalone for testing.

Thanks.

-- 
Dmitry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64/cache: silence -Woverride-init warnings
From: Qian Cai @ 2019-08-08 22:18 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Will Deacon, Linux List Kernel Mailing,
	linux-arm-kernel, clang-built-linux
In-Reply-To: <20190808103808.GC46901@lakrids.cambridge.arm.com>



> On Aug 8, 2019, at 6:38 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Wed, Aug 07, 2019 at 11:29:16PM -0400, Qian Cai wrote:
>> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
>> VIVT I-caches") introduced some compiation warnings from GCC (and
>> Clang) with -Winitializer-overrides),
>> 
>> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
>> overwritten [-Woverride-init]
>> [ICACHE_POLICY_VIPT]  = "VIPT",
>>                        ^~~~~~
>> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
>> 'icache_policy_str[2]')
>> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
>> overwritten [-Woverride-init]
>> [ICACHE_POLICY_PIPT]  = "PIPT",
>>                        ^~~~~~
>> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
>> 'icache_policy_str[3]')
>> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
>> overwritten [-Woverride-init]
>> [ICACHE_POLICY_VPIPT]  = "VPIPT",
>>                         ^~~~~~~
>> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
>> 'icache_policy_str[0]')
>> 
>> because it initializes icache_policy_str[0 ... 3] twice. Since
>> arm64 developers are keen to keep the style of initializing a static
>> array with a non-zero pattern first, just disable those warnings for
>> both GCC and Clang of this file.
>> 
>> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
>> Signed-off-by: Qian Cai <cai@lca.pw>
> 
> This is _not_ a fix, and should not require backporting to stable trees.

From my experience, the stable AI will pick up whatever they want to backport
not matter if there Is a “Fixes” tag or not unless it is one of those subsystems like
Networking that exclusively manually flag for. backporting by the maintainer.  

> 
> What about all the other instances that we have in mainline?

I have not had a chance to review all instances yet. It is not unusually to fix one
warning at a time, and then go on fixing some more if time permit.

> 
> I really don't think that we need to go down this road; we're just going
> to end up adding this to every file that happens to include a header
> using this scheme…

How about disable them this way in a top level like arch/arm64/Makefile or
arch/arm64/kernel/Makefile? Therefore, there is no need to add this to
every file, but with a drawback that it could miss a few real issues there
in the future which probably not many people are checking for them of
the arm64 subsystem nowadays.

> 
> Please just turn this off by default for clang.

As mentioned before, it is very valuable to run “make W=1” given it found
many real developer mistakes which will enable “-Woverride-init” for both
compilers. Even “-Woverride-init” itself is useful find real issues as in,

ae5e033d65a (“mfd: rk808: Fix RK818_IRQ_DISCHG_ILIM initializer”)
32df34d875bb (“[media] rc: img-ir: jvc: Remove unused no-leader timings”)

Especially, to find redundant initializations in large structures. e.g.,

e6ea0b917875 (“[media] dvb_frontend: Don't declare values twice at a table”)

It is important to keep the noise-level as low as possible by keeping the
amount of false positives under control to be truly benefit from those
valuable compiler warnings. 

> 
> If we want to enable this, we need a mechanism to permit overridable
> assignments as we use range initializers for.

I am not sure that it is worth filling a RFE for compilers of that feature.
I feel like the range initializers just another way to initialize an array, and
 it is just easier to make mistakes with unintended double-initializations.
The compiler developers probably recommend to enforce more of
“-Woverride-init” for  the range initializers rather than permitting it.

> 
> Thanks,
> Mark.
> 
>> ---
>> arch/arm64/kernel/Makefile | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 478491f07b4f..397ed5f7be1e 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -11,6 +11,9 @@ CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE)
>> CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE)
>> CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE)
>> 
>> +CFLAGS_cpuinfo.o += $(call cc-disable-warning, override-init)
>> +CFLAGS_cpuinfo.o += $(call cc-disable-warning, initializer-overrides)
>> +
>> # Object file lists.
>> obj-y			:= debug-monitors.o entry.o irq.o fpsimd.o		\
>> 			   entry-fpsimd.o process.o ptrace.o setup.o signal.o	\
>> -- 
>> 2.20.1 (Apple Git-117)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] phy: Change the configuration interface param to void* to make it more general
From: kbuild test robot @ 2019-08-08 22:01 UTC (permalink / raw)
  To: Zeng Tao
  Cc: Sakari Ailus, Maxime Ripard, linux-kernel, kishon,
	Paul Kocialkowski, Chen-Yu Tsai, kbuild-all, prime.zeng,
	linux-arm-kernel
In-Reply-To: <1562868255-31467-1-git-send-email-prime.zeng@hisilicon.com>

Hi Zeng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc3 next-20190808]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Zeng-Tao/phy-Change-the-configuration-interface-param-to-void-to-make-it-more-general/20190713-213420
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
   include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
>> drivers/gpu/drm/bridge/cdns-dsi.c:609:73: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
   drivers/gpu/drm/bridge/cdns-dsi.c:784:73: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
--
   include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
   include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:165:22: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:169:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:173:17: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:200:17: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:206:9: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:211:9: sparse: sparse: using member 'lp_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:216:9: sparse: sparse: using member 'lp_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:220:26: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:221:17: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:227:22: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:230:37: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:237:26: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:238:17: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:244:43: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:247:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:251:29: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:255:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:264:22: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:266:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:268:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:270:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:272:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:274:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
>> drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:326:53: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:356:54: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts

vim +/mipi_dphy +609 drivers/gpu/drm/bridge/cdns-dsi.c

4dad3e7f12f702 Maxime Ripard   2019-01-21  602  
4dad3e7f12f702 Maxime Ripard   2019-01-21  603  static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
4dad3e7f12f702 Maxime Ripard   2019-01-21  604  			       const struct drm_display_mode *mode,
4dad3e7f12f702 Maxime Ripard   2019-01-21  605  			       struct cdns_dsi_cfg *dsi_cfg,
4dad3e7f12f702 Maxime Ripard   2019-01-21  606  			       bool mode_valid_check)
4dad3e7f12f702 Maxime Ripard   2019-01-21  607  {
4dad3e7f12f702 Maxime Ripard   2019-01-21  608  	struct cdns_dsi_output *output = &dsi->output;
fced5a364dee9d Maxime Ripard   2019-01-21 @609  	struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
4dad3e7f12f702 Maxime Ripard   2019-01-21  610  	unsigned long dsi_hss_hsa_hse_hbp;
4dad3e7f12f702 Maxime Ripard   2019-01-21  611  	unsigned int nlanes = output->dev->lanes;
4dad3e7f12f702 Maxime Ripard   2019-01-21  612  	int ret;
4dad3e7f12f702 Maxime Ripard   2019-01-21  613  
4dad3e7f12f702 Maxime Ripard   2019-01-21  614  	ret = cdns_dsi_mode2cfg(dsi, mode, dsi_cfg, mode_valid_check);
4dad3e7f12f702 Maxime Ripard   2019-01-21  615  	if (ret)
4dad3e7f12f702 Maxime Ripard   2019-01-21  616  		return ret;
4dad3e7f12f702 Maxime Ripard   2019-01-21  617  
fced5a364dee9d Maxime Ripard   2019-01-21  618  	phy_mipi_dphy_get_default_config(mode->crtc_clock * 1000,
fced5a364dee9d Maxime Ripard   2019-01-21  619  					 mipi_dsi_pixel_format_to_bpp(output->dev->format),
fced5a364dee9d Maxime Ripard   2019-01-21  620  					 nlanes, phy_cfg);
fced5a364dee9d Maxime Ripard   2019-01-21  621  
fced5a364dee9d Maxime Ripard   2019-01-21  622  	ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, mode, mode_valid_check);
fced5a364dee9d Maxime Ripard   2019-01-21  623  	if (ret)
fced5a364dee9d Maxime Ripard   2019-01-21  624  		return ret;
fced5a364dee9d Maxime Ripard   2019-01-21  625  
fced5a364dee9d Maxime Ripard   2019-01-21  626  	ret = phy_validate(dsi->dphy, PHY_MODE_MIPI_DPHY, 0, &output->phy_opts);
4dad3e7f12f702 Maxime Ripard   2019-01-21  627  	if (ret)
4dad3e7f12f702 Maxime Ripard   2019-01-21  628  		return ret;
4dad3e7f12f702 Maxime Ripard   2019-01-21  629  
4dad3e7f12f702 Maxime Ripard   2019-01-21  630  	dsi_hss_hsa_hse_hbp = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
4dad3e7f12f702 Maxime Ripard   2019-01-21  631  	if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
4dad3e7f12f702 Maxime Ripard   2019-01-21  632  		dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
e19233955d9e9a Boris Brezillon 2018-04-21  633  
e19233955d9e9a Boris Brezillon 2018-04-21  634  	/*
e19233955d9e9a Boris Brezillon 2018-04-21  635  	 * Make sure DPI(HFP) > DSI(HSS+HSA+HSE+HBP) to guarantee that the FIFO
e19233955d9e9a Boris Brezillon 2018-04-21  636  	 * is empty before we start a receiving a new line on the DPI
e19233955d9e9a Boris Brezillon 2018-04-21  637  	 * interface.
e19233955d9e9a Boris Brezillon 2018-04-21  638  	 */
fced5a364dee9d Maxime Ripard   2019-01-21  639  	if ((u64)phy_cfg->hs_clk_rate *
4dad3e7f12f702 Maxime Ripard   2019-01-21  640  	    mode_to_dpi_hfp(mode, mode_valid_check) * nlanes <
e19233955d9e9a Boris Brezillon 2018-04-21  641  	    (u64)dsi_hss_hsa_hse_hbp *
e19233955d9e9a Boris Brezillon 2018-04-21  642  	    (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000)
e19233955d9e9a Boris Brezillon 2018-04-21  643  		return -EINVAL;
e19233955d9e9a Boris Brezillon 2018-04-21  644  
e19233955d9e9a Boris Brezillon 2018-04-21  645  	return 0;
e19233955d9e9a Boris Brezillon 2018-04-21  646  }
e19233955d9e9a Boris Brezillon 2018-04-21  647  

:::::: The code at line 609 was first introduced by commit
:::::: fced5a364dee9d3a3ed1e3290ea3b83984b78007 drm/bridge: cdns: Convert to phy framework

:::::: TO: Maxime Ripard <maxime.ripard@bootlin.com>
:::::: CC: Maxime Ripard <maxime.ripard@bootlin.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v8 09/14] media: rkisp1: add rockchip isp1 core driver
From: Helen Koike @ 2019-08-08 21:59 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: devicetree, eddie.cai.linux, kernel, heiko, jacob2.chen,
	jeffy.chen, zyc, linux-kernel, tfiga, linux-rockchip, Allon Huang,
	Jacob Chen, hans.verkuil, laurent.pinchart, zhengsq, mchehab,
	ezequiel, linux-arm-kernel, linux-media
In-Reply-To: <20190807152727.GN21370@paasikivi.fi.intel.com>

Hi Sakari,

Thanks for your review, just some comments/questions below:

On 8/7/19 12:27 PM, Sakari Ailus wrote:
> Hi Helen,
> 
> On Tue, Jul 30, 2019 at 03:42:51PM -0300, Helen Koike wrote:
>> From: Jacob Chen <jacob2.chen@rock-chips.com>
>>
>> Add the core driver for rockchip isp1.
>>
>> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>> Signed-off-by: Yichong Zhong <zyc@rock-chips.com>
>> Signed-off-by: Jacob Chen <cc@rock-chips.com>
>> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> Signed-off-by: Allon Huang <allon.huang@rock-chips.com>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> [fixed compilation and run time errors regarding new v4l2 async API]
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> [Add missing module device table]
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> [update for upstream]
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>
>> ---
>>
>> Changes in v8: None
>> Changes in v7:
>> - VIDEO_ROCKCHIP_ISP1 selects VIDEOBUF2_VMALLOC
>> - add PHY_ROCKCHIP_DPHY as a dependency for VIDEO_ROCKCHIP_ISP1
>> - Fix compilation and runtime errors due to bitrotting
>> The code has bit-rotten since March 2018, fix compilation errors.
>> The new V4L2 async notifier API requires notifiers to be initialized by
>> a call to v4l2_async_notifier_init() before being used, do so.
>> - Add missing module device table
>> - use clk_bulk framework
>> - add missing notifiers cleanups
>> - s/strlcpy/strscpy
>> - normalize bus_info name
>> - fix s_stream error path, stream_cnt wans't being decremented properly
>> - use devm_platform_ioremap_resource() helper
>> - s/deice/device
>> - redesign: remove mipi/csi subdevice, sensors connect directly to the
>> isp subdevice in the media topology now.
>> - remove "saved_state" member from rkisp1_stream struct
>> - Reverse the order of MIs
>> - Simplify MI interrupt handling
>> Rather than adding unnecessary indirection, just use stream index to
>> handle MI interrupt enable/disable/clear, since the stream index matches
>> the order of bits now, thanks to previous patch. While at it, remove
>> some dead code.
>> - code styling and checkpatch fixes
>>
>>  drivers/media/platform/Kconfig                |  12 +
>>  drivers/media/platform/Makefile               |   1 +
>>  drivers/media/platform/rockchip/isp1/Makefile |   7 +
>>  drivers/media/platform/rockchip/isp1/common.h | 101 +++
>>  drivers/media/platform/rockchip/isp1/dev.c    | 675 ++++++++++++++++++
>>  drivers/media/platform/rockchip/isp1/dev.h    |  97 +++
>>  6 files changed, 893 insertions(+)
>>  create mode 100644 drivers/media/platform/rockchip/isp1/Makefile
>>  create mode 100644 drivers/media/platform/rockchip/isp1/common.h
>>  create mode 100644 drivers/media/platform/rockchip/isp1/dev.c
>>  create mode 100644 drivers/media/platform/rockchip/isp1/dev.h
>>
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index 89555f9a813f..e0e98937c565 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -106,6 +106,18 @@ config VIDEO_QCOM_CAMSS
>>  	select VIDEOBUF2_DMA_SG
>>  	select V4L2_FWNODE
>>  
>> +config VIDEO_ROCKCHIP_ISP1
>> +	tristate "Rockchip Image Signal Processing v1 Unit driver"
>> +	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> +	depends on ARCH_ROCKCHIP || COMPILE_TEST
>> +	select VIDEOBUF2_DMA_CONTIG
>> +	select VIDEOBUF2_VMALLOC
>> +	select V4L2_FWNODE
>> +	select PHY_ROCKCHIP_DPHY
>> +	default n
>> +	---help---
>> +	  Support for ISP1 on the rockchip SoC.
>> +
>>  config VIDEO_S3C_CAMIF
>>  	tristate "Samsung S3C24XX/S3C64XX SoC Camera Interface driver"
>>  	depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
>> index 7cbbd925124c..f9fcf8e7c513 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -69,6 +69,7 @@ obj-$(CONFIG_VIDEO_RENESAS_FDP1)	+= rcar_fdp1.o
>>  obj-$(CONFIG_VIDEO_RENESAS_JPU)		+= rcar_jpu.o
>>  obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1/
>>  
>> +obj-$(CONFIG_VIDEO_ROCKCHIP_ISP1)	+= rockchip/isp1/
>>  obj-$(CONFIG_VIDEO_ROCKCHIP_RGA)	+= rockchip/rga/
>>  
>>  obj-y	+= omap/
>> diff --git a/drivers/media/platform/rockchip/isp1/Makefile b/drivers/media/platform/rockchip/isp1/Makefile
>> new file mode 100644
>> index 000000000000..72706e80fc8b
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/Makefile
>> @@ -0,0 +1,7 @@
>> +obj-$(CONFIG_VIDEO_ROCKCHIP_ISP1) += 	rockchip-isp1.o
>> +rockchip-isp1-objs 	   += 	rkisp1.o \
>> +				dev.o \
>> +				regs.o \
>> +				isp_stats.o \
>> +				isp_params.o \
>> +				capture.o
>> diff --git a/drivers/media/platform/rockchip/isp1/common.h b/drivers/media/platform/rockchip/isp1/common.h
>> new file mode 100644
>> index 000000000000..606ce2793546
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/common.h
>> @@ -0,0 +1,101 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Rockchip isp1 driver
>> + *
>> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#ifndef _RKISP1_COMMON_H
>> +#define _RKISP1_COMMON_H
>> +
>> +#include <linux/mutex.h>
>> +#include <media/media-device.h>
>> +#include <media/media-entity.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/videobuf2-v4l2.h>
>> +
>> +#define RKISP1_DEFAULT_WIDTH		800
>> +#define RKISP1_DEFAULT_HEIGHT		600
>> +
>> +#define RKISP1_MAX_STREAM		2
>> +#define RKISP1_STREAM_MP		0
>> +#define RKISP1_STREAM_SP		1
>> +
>> +#define RKISP1_PLANE_Y			0
>> +#define RKISP1_PLANE_CB			1
>> +#define RKISP1_PLANE_CR			2
>> +
>> +enum rkisp1_sd_type {
>> +	RKISP1_SD_SENSOR,
>> +	RKISP1_SD_PHY_CSI,
>> +	RKISP1_SD_VCM,
>> +	RKISP1_SD_FLASH,
>> +	RKISP1_SD_MAX,
>> +};
> 
> I wonder if this is a leftover from the driver development time. Same goes
> for the subdevs field in struct rkisp1_device.

It is a left over, I'm removing them for the next version, thanks.

> 
>> +
>> +/* One structure per video node */
>> +struct rkisp1_vdev_node {
>> +	struct vb2_queue buf_queue;
>> +	/* vfd lock */
>> +	struct mutex vlock;
>> +	struct video_device vdev;
>> +	struct media_pad pad;
>> +};
>> +
>> +enum rkisp1_fmt_pix_type {
>> +	FMT_YUV,
>> +	FMT_RGB,
>> +	FMT_BAYER,
>> +	FMT_JPEG,
>> +	FMT_MAX
>> +};
>> +
>> +enum rkisp1_fmt_raw_pat_type {
>> +	RAW_RGGB = 0,
>> +	RAW_GRBG,
>> +	RAW_GBRG,
>> +	RAW_BGGR,
>> +};
>> +
>> +struct rkisp1_buffer {
>> +	struct vb2_v4l2_buffer vb;
>> +	struct list_head queue;
>> +	union {
>> +		u32 buff_addr[VIDEO_MAX_PLANES];
>> +		void *vaddr[VIDEO_MAX_PLANES];
>> +	};
>> +};
>> +
>> +struct rkisp1_dummy_buffer {
>> +	void *vaddr;
>> +	dma_addr_t dma_addr;
>> +	u32 size;
>> +};
>> +
>> +extern int rkisp1_debug;
>> +
>> +static inline
>> +struct rkisp1_vdev_node *vdev_to_node(struct video_device *vdev)
>> +{
>> +	return container_of(vdev, struct rkisp1_vdev_node, vdev);
>> +}
>> +
>> +static inline struct rkisp1_vdev_node *queue_to_node(struct vb2_queue *q)
>> +{
>> +	return container_of(q, struct rkisp1_vdev_node, buf_queue);
>> +}
>> +
>> +static inline struct rkisp1_buffer *to_rkisp1_buffer(struct vb2_v4l2_buffer *vb)
>> +{
>> +	return container_of(vb, struct rkisp1_buffer, vb);
>> +}
>> +
>> +static inline struct vb2_queue *to_vb2_queue(struct file *file)
>> +{
>> +	struct rkisp1_vdev_node *vnode = video_drvdata(file);
>> +
>> +	return &vnode->buf_queue;
>> +}
>> +
>> +#endif /* _RKISP1_COMMON_H */
>> diff --git a/drivers/media/platform/rockchip/isp1/dev.c b/drivers/media/platform/rockchip/isp1/dev.c
>> new file mode 100644
>> index 000000000000..2b4a67e1a3b5
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/dev.c
>> @@ -0,0 +1,675 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Rockchip isp1 driver
>> + *
>> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/pinctrl/consumer.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/phy/phy-mipi-dphy.h>
>> +
>> +#include "common.h"
>> +#include "regs.h"
>> +
>> +struct isp_match_data {
>> +	const char * const *clks;
>> +	int size;
> 
> unsigned int

ack

> 
>> +};
>> +
>> +struct sensor_async_subdev {
>> +	struct v4l2_async_subdev asd;
>> +	struct v4l2_mbus_config mbus;
>> +	unsigned int lanes;
>> +};
>> +
>> +int rkisp1_debug;
>> +module_param_named(debug, rkisp1_debug, int, 0644);
>> +MODULE_PARM_DESC(debug, "Debug level (0-1)");
> 
> Have you thought of using dynamic debug instead?

right, this is being used in v4l2_dbg(), which I can replace by dev_dbg()
that is already covered by dynamic debug iirc.
Should I also replace v4l2_err() by dev_err() (I always get confused by
which log function I should use).

> 
>> +
>> +/**************************** pipeline operations******************************/
>> +
>> +static int __isp_pipeline_prepare(struct rkisp1_pipeline *p,
>> +				  struct media_entity *me)
>> +{
>> +	struct rkisp1_device *dev = container_of(p, struct rkisp1_device, pipe);
>> +	struct v4l2_subdev *sd;
>> +	unsigned int i;
>> +
>> +	p->num_subdevs = 0;
>> +	memset(p->subdevs, 0, sizeof(p->subdevs));
>> +
>> +	while (1) {
>> +		struct media_pad *pad = NULL;
>> +
>> +		/* Find remote source pad */
>> +		for (i = 0; i < me->num_pads; i++) {
>> +			struct media_pad *spad = &me->pads[i];
>> +
>> +			if (!(spad->flags & MEDIA_PAD_FL_SINK))
>> +				continue;
>> +			pad = media_entity_remote_pad(spad);
>> +			if (pad)
>> +				break;
>> +		}
>> +
>> +		if (!pad)
>> +			break;
>> +
>> +		sd = media_entity_to_v4l2_subdev(pad->entity);
>> +		if (sd != &dev->isp_sdev.sd)
>> +			p->subdevs[p->num_subdevs++] = sd;
> 
> How do you make sure you don't overrun the array?

Because the maximum path the topology can have is:
sensor->rkisp->capture

> 
> Instead, I'd avoid maintaining the array in the first place --- the same
> information is available from the MC framework data structures --- see e.g.
> the omap3isp driver.

If I understand correctly, omap3isp navigates through the topology in the same way,
but it just don't save in an array, but I reuse this information in other places,
mostly for power up/down (see below why I don't use v4l2_pipeline_pm_use())

> 
>> +
>> +		me = &sd->entity;
>> +		if (me->num_pads == 1)
>> +			break;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int __subdev_set_power(struct v4l2_subdev *sd, int on)
>> +{
>> +	int ret;
>> +
>> +	if (!sd)
>> +		return -ENXIO;
>> +
>> +	ret = v4l2_subdev_call(sd, core, s_power, on);
>> +
>> +	return ret != -ENOIOCTLCMD ? ret : 0;
>> +}
>> +
>> +static int __isp_pipeline_s_power(struct rkisp1_pipeline *p, bool on)
> 
> Could you instead use v4l2_pipeline_pm_use()?

Unless I misunderstood this (which is very likely), v4l2_pipeline_pm_use() calls pipeline_pm_power(),
that applies power change to all entities in a pipeline, but if I have two sensors
connected to the ISP, one with link enabled and the other with a disabled link,
I don't want to power both sensors on, just the one participating in that stream. And
if I call v4l2_pipeline_pm_use() it will power on both, which is not what I want.

Does it make sense?

> 
>> +{
>> +	struct rkisp1_device *dev = container_of(p, struct rkisp1_device, pipe);
>> +	int i, ret;
>> +
>> +	if (on) {
>> +		__subdev_set_power(&dev->isp_sdev.sd, true);
>> +
>> +		for (i = p->num_subdevs - 1; i >= 0; --i) {
>> +			ret = __subdev_set_power(p->subdevs[i], true);
>> +			if (ret < 0 && ret != -ENXIO)
>> +				goto err_power_off;
>> +		}
>> +	} else {
>> +		for (i = 0; i < p->num_subdevs; ++i)
>> +			__subdev_set_power(p->subdevs[i], false);
>> +
>> +		__subdev_set_power(&dev->isp_sdev.sd, false);
>> +	}
>> +
>> +	return 0;
>> +
>> +err_power_off:
>> +	for (++i; i < p->num_subdevs; ++i)
>> +		__subdev_set_power(p->subdevs[i], false);
>> +	__subdev_set_power(&dev->isp_sdev.sd, true);
>> +	return ret;
>> +}
>> +
>> +static int rkisp1_pipeline_open(struct rkisp1_pipeline *p,
>> +				struct media_entity *me,
>> +				bool prepare)
>> +{
>> +	int ret;
>> +
>> +	if (WARN_ON(!p || !me))
>> +		return -EINVAL;
>> +	if (atomic_inc_return(&p->power_cnt) > 1)
>> +		return 0;
>> +
>> +	/* go through media graphic and get subdevs */
>> +	if (prepare)
>> +		__isp_pipeline_prepare(p, me);
>> +
>> +	if (!p->num_subdevs)
>> +		return -EINVAL;
>> +
>> +	ret = __isp_pipeline_s_power(p, 1);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static int rkisp1_pipeline_close(struct rkisp1_pipeline *p)
>> +{
>> +	int ret;
>> +
>> +	if (atomic_dec_return(&p->power_cnt) > 0)
>> +		return 0;
>> +	ret = __isp_pipeline_s_power(p, 0);
>> +
>> +	return ret == -ENXIO ? 0 : ret;
>> +}
>> +
>> +/*
>> + * stream-on order: isp_subdev, mipi dphy, sensor
>> + * stream-off order: mipi dphy, sensor, isp_subdev
>> + */
>> +static int rkisp1_pipeline_set_stream(struct rkisp1_pipeline *p, bool on)
>> +{
>> +	struct rkisp1_device *dev = container_of(p, struct rkisp1_device, pipe);
>> +	int i, ret;
>> +
>> +	if ((on && atomic_inc_return(&p->stream_cnt) > 1) ||
>> +	    (!on && atomic_dec_return(&p->stream_cnt) > 0))
>> +		return 0;
>> +
>> +	if (on) {
>> +		ret = v4l2_subdev_call(&dev->isp_sdev.sd, video, s_stream,
>> +				       true);
>> +		if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) {
>> +			v4l2_err(&dev->v4l2_dev,
>> +				 "s_stream failed on subdevice %s (%d)\n",
>> +				 dev->isp_sdev.sd.name,
>> +				 ret);
>> +			atomic_dec(&p->stream_cnt);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	/* phy -> sensor */
>> +	for (i = 0; i < p->num_subdevs; ++i) {
>> +		ret = v4l2_subdev_call(p->subdevs[i], video, s_stream, on);
>> +		if (on && ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> +			goto err_stream_off;
> 
> You should stop after the first external sub-device.
> 
> It seems even the omap3isp driver doesn't do that. It's not easy to spot
> such issues indeed.

I'm not sure I understand, what do you call an external sub-device? Is the sensor
an external subdevice?

> 
>> +	}
>> +
>> +	if (!on)
>> +		v4l2_subdev_call(&dev->isp_sdev.sd, video, s_stream, false);
>> +
>> +	return 0;
>> +
>> +err_stream_off:
>> +	for (--i; i >= 0; --i)
>> +		v4l2_subdev_call(p->subdevs[i], video, s_stream, false);
>> +	v4l2_subdev_call(&dev->isp_sdev.sd, video, s_stream, false);
>> +	atomic_dec(&p->stream_cnt);
>> +	return ret;
>> +}
>> +
>> +/***************************** media controller *******************************/
>> +/* See http://opensource.rock-chips.com/wiki_Rockchip-isp1 for Topology */
> 
> The host appears to be down, or there's a routing problem. Unless this is
> fixed, having such a URL here doesn't do much good. :-I

This is a left over, sorry about that.
I can access the URL now. I'll try to get some of the docs and move to the kernel docs.

> 
>> +
>> +static int rkisp1_create_links(struct rkisp1_device *dev)
>> +{
>> +	struct media_entity *source, *sink;
>> +	struct rkisp1_sensor *sensor;
>> +	unsigned int flags, pad;
>> +	int ret;
>> +
>> +	/* sensor links(or mipi-phy) */
>> +	list_for_each_entry(sensor, &dev->sensors, list) {
>> +		for (pad = 0; pad < sensor->sd->entity.num_pads; pad++)
>> +			if (sensor->sd->entity.pads[pad].flags &
>> +				MEDIA_PAD_FL_SOURCE)
>> +				break;
> 
> Could you use media_entity_get_fwnode_pad() instead?

Yes, I didn't know about it actually, thanks for that, looks cleaner (I'll send in
the next version).

> 
>> +
>> +		if (pad == sensor->sd->entity.num_pads) {
>> +			dev_err(dev->dev,
>> +				"failed to find src pad for %s\n",
>> +				sensor->sd->name);
>> +
>> +			return -ENXIO;
>> +		}
>> +
>> +		ret = media_create_pad_link(
>> +				&sensor->sd->entity, pad,
>> +				&dev->isp_sdev.sd.entity,
>> +				RKISP1_ISP_PAD_SINK,
>> +				list_is_first(&sensor->list, &dev->sensors) ?
>> +				MEDIA_LNK_FL_ENABLED : 0);
>> +		if (ret) {
>> +			dev_err(dev->dev,
>> +				"failed to create link for %s\n",
>> +				sensor->sd->name);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	/* params links */
>> +	source = &dev->params_vdev.vnode.vdev.entity;
>> +	sink = &dev->isp_sdev.sd.entity;
>> +	flags = MEDIA_LNK_FL_ENABLED;
>> +	ret = media_create_pad_link(source, 0, sink,
>> +				       RKISP1_ISP_PAD_SINK_PARAMS, flags);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* create isp internal links */
>> +	/* SP links */
>> +	source = &dev->isp_sdev.sd.entity;
>> +	sink = &dev->stream[RKISP1_STREAM_SP].vnode.vdev.entity;
>> +	ret = media_create_pad_link(source, RKISP1_ISP_PAD_SOURCE_PATH,
>> +				       sink, 0, flags);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* MP links */
>> +	source = &dev->isp_sdev.sd.entity;
>> +	sink = &dev->stream[RKISP1_STREAM_MP].vnode.vdev.entity;
>> +	ret = media_create_pad_link(source, RKISP1_ISP_PAD_SOURCE_PATH,
>> +				       sink, 0, flags);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	/* 3A stats links */
>> +	source = &dev->isp_sdev.sd.entity;
>> +	sink = &dev->stats_vdev.vnode.vdev.entity;
>> +	return media_create_pad_link(source, RKISP1_ISP_PAD_SOURCE_STATS,
>> +					sink, 0, flags);
> 
> Indentation. Same for the calls to the same function above.

ack

> 
>> +}
>> +
>> +static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>> +				 struct v4l2_subdev *sd,
>> +				 struct v4l2_async_subdev *asd)
>> +{
>> +	struct rkisp1_device *isp_dev = container_of(notifier,
>> +						     struct rkisp1_device,
>> +						     notifier);
>> +	struct sensor_async_subdev *s_asd = container_of(asd,
>> +					struct sensor_async_subdev, asd);
>> +	struct rkisp1_sensor *sensor;
>> +
>> +	sensor = devm_kzalloc(isp_dev->dev, sizeof(*sensor), GFP_KERNEL);
>> +	if (!sensor)
>> +		return -ENOMEM;
>> +
>> +	sensor->lanes = s_asd->lanes;
>> +	sensor->mbus = s_asd->mbus;
>> +	sensor->sd = sd;
>> +	sensor->dphy = devm_phy_get(isp_dev->dev, "dphy");
>> +	if (IS_ERR(sensor->dphy)) {
>> +		if (PTR_ERR(sensor->dphy) != -EPROBE_DEFER)
>> +			dev_err(isp_dev->dev, "Couldn't get the MIPI D-PHY\n");
>> +		return PTR_ERR(sensor->dphy);
>> +	}
>> +	phy_init(sensor->dphy);
>> +
>> +	list_add(&sensor->list, &isp_dev->sensors);
> 
> In general, maintaining the information on the external subdevs on your own
> adds complexity to the driver. You can get the information when you need it
> from the data structures maintained by MC (see e.g. the omap3isp driver for
> examples).
> 
>> +
>> +	return 0;
>> +}
>> +
>> +static struct rkisp1_sensor *sd_to_sensor(struct rkisp1_device *dev,
>> +					  struct v4l2_subdev *sd)
>> +{
>> +	struct rkisp1_sensor *sensor;
>> +
>> +	list_for_each_entry(sensor, &dev->sensors, list)
>> +		if (sensor->sd == sd)
>> +			return sensor;
>> +
>> +	return NULL;
>> +}
>> +
>> +static void subdev_notifier_unbind(struct v4l2_async_notifier *notifier,
>> +				   struct v4l2_subdev *sd,
>> +				   struct v4l2_async_subdev *asd)
>> +{
>> +	struct rkisp1_device *isp_dev = container_of(notifier,
>> +						     struct rkisp1_device,
>> +						     notifier);
>> +	struct rkisp1_sensor *sensor = sd_to_sensor(isp_dev, sd);
>> +
>> +	/* TODO: check if a lock is required here */
>> +	list_del(&sensor->list);
>> +
>> +	phy_exit(sensor->dphy);
>> +}
>> +
>> +static int subdev_notifier_complete(struct v4l2_async_notifier *notifier)
>> +{
>> +	struct rkisp1_device *dev = container_of(notifier, struct rkisp1_device,
>> +						 notifier);
>> +	int ret;
>> +
>> +	mutex_lock(&dev->media_dev.graph_mutex);
>> +	ret = rkisp1_create_links(dev);
>> +	if (ret < 0)
>> +		goto unlock;
>> +	ret = v4l2_device_register_subdev_nodes(&dev->v4l2_dev);
>> +	if (ret < 0)
>> +		goto unlock;
>> +
>> +	v4l2_info(&dev->v4l2_dev, "Async subdev notifier completed\n");
>> +
>> +unlock:
>> +	mutex_unlock(&dev->media_dev.graph_mutex);
>> +	return ret;
>> +}
>> +
>> +static int rkisp1_fwnode_parse(struct device *dev,
>> +			       struct v4l2_fwnode_endpoint *vep,
>> +			       struct v4l2_async_subdev *asd)
>> +{
>> +	struct sensor_async_subdev *s_asd =
>> +			container_of(asd, struct sensor_async_subdev, asd);
>> +
>> +	if (vep->bus_type != V4L2_MBUS_CSI2_DPHY) {
>> +		dev_err(dev, "Only CSI2 bus type is currently supported\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (vep->base.port != 0) {
>> +		dev_err(dev, "The ISP has only port 0\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	s_asd->mbus.type = vep->bus_type;
>> +	s_asd->mbus.flags = vep->bus.mipi_csi2.flags;
>> +	s_asd->lanes = vep->bus.mipi_csi2.num_data_lanes;
>> +
>> +	switch (vep->bus.mipi_csi2.num_data_lanes) {
>> +	case 1:
>> +		s_asd->mbus.flags |= V4L2_MBUS_CSI2_1_LANE;
>> +		break;
>> +	case 2:
>> +		s_asd->mbus.flags |= V4L2_MBUS_CSI2_2_LANE;
>> +		break;
>> +	case 3:
>> +		s_asd->mbus.flags |= V4L2_MBUS_CSI2_3_LANE;
>> +		break;
>> +	case 4:
>> +		s_asd->mbus.flags |= V4L2_MBUS_CSI2_4_LANE;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct v4l2_async_notifier_operations subdev_notifier_ops = {
>> +	.bound = subdev_notifier_bound,
>> +	.unbind = subdev_notifier_unbind,
>> +	.complete = subdev_notifier_complete,
>> +};
>> +
>> +static int isp_subdev_notifier(struct rkisp1_device *isp_dev)
>> +{
>> +	struct v4l2_async_notifier *ntf = &isp_dev->notifier;
>> +	struct device *dev = isp_dev->dev;
>> +	int ret;
>> +
>> +	v4l2_async_notifier_init(ntf);
>> +
>> +	ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>> +		dev, ntf, sizeof(struct sensor_async_subdev), 0,
>> +		rkisp1_fwnode_parse);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (list_empty(&ntf->asd_list))
>> +		return -ENODEV;	/* no endpoint */
>> +
>> +	ntf->ops = &subdev_notifier_ops;
>> +
>> +	return v4l2_async_notifier_register(&isp_dev->v4l2_dev, ntf);
>> +}
>> +
>> +/***************************** platform device *******************************/
>> +
>> +static int rkisp1_register_platform_subdevs(struct rkisp1_device *dev)
>> +{
>> +	int ret;
>> +
>> +	ret = rkisp1_register_isp_subdev(dev, &dev->v4l2_dev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = rkisp1_register_stream_vdevs(dev);
>> +	if (ret < 0)
>> +		goto err_unreg_isp_subdev;
>> +
>> +	ret = rkisp1_register_stats_vdev(&dev->stats_vdev, &dev->v4l2_dev, dev);
>> +	if (ret < 0)
>> +		goto err_unreg_stream_vdev;
>> +
>> +	ret = rkisp1_register_params_vdev(&dev->params_vdev, &dev->v4l2_dev,
>> +					  dev);
>> +	if (ret < 0)
>> +		goto err_unreg_stats_vdev;
>> +
>> +	ret = isp_subdev_notifier(dev);
>> +	if (ret < 0) {
>> +		v4l2_err(&dev->v4l2_dev,
>> +			 "Failed to register subdev notifier(%d)\n", ret);
>> +		goto err_unreg_params_vdev;
>> +	}
>> +
>> +	return 0;
>> +err_unreg_params_vdev:
>> +	rkisp1_unregister_params_vdev(&dev->params_vdev);
>> +err_unreg_stats_vdev:
>> +	rkisp1_unregister_stats_vdev(&dev->stats_vdev);
>> +err_unreg_stream_vdev:
>> +	rkisp1_unregister_stream_vdevs(dev);
>> +err_unreg_isp_subdev:
>> +	rkisp1_unregister_isp_subdev(dev);
>> +	return ret;
>> +}
>> +
>> +static const char * const rk3399_isp_clks[] = {
>> +	"clk_isp",
>> +	"aclk_isp",
>> +	"hclk_isp",
>> +	"aclk_isp_wrap",
>> +	"hclk_isp_wrap",
>> +};
>> +
>> +static const char * const rk3288_isp_clks[] = {
>> +	"clk_isp",
>> +	"aclk_isp",
>> +	"hclk_isp",
>> +	"pclk_isp_in",
>> +	"sclk_isp_jpe",
>> +};
>> +
>> +static const struct isp_match_data rk3288_isp_clk_data = {
>> +	.clks = rk3288_isp_clks,
>> +	.size = ARRAY_SIZE(rk3288_isp_clks),
>> +};
>> +
>> +static const struct isp_match_data rk3399_isp_clk_data = {
>> +	.clks = rk3399_isp_clks,
>> +	.size = ARRAY_SIZE(rk3399_isp_clks),
>> +};
>> +
>> +static const struct of_device_id rkisp1_plat_of_match[] = {
>> +	{
>> +		.compatible = "rockchip,rk3288-cif-isp",
>> +		.data = &rk3288_isp_clk_data,
>> +	}, {
>> +		.compatible = "rockchip,rk3399-cif-isp",
>> +		.data = &rk3399_isp_clk_data,
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, rkisp1_plat_of_match);
>> +
>> +static irqreturn_t rkisp1_irq_handler(int irq, void *ctx)
>> +{
>> +	struct device *dev = ctx;
>> +	struct rkisp1_device *rkisp1_dev = dev_get_drvdata(dev);
>> +	unsigned int mis_val;
>> +
>> +	mis_val = readl(rkisp1_dev->base_addr + CIF_ISP_MIS);
>> +	if (mis_val)
>> +		rkisp1_isp_isr(mis_val, rkisp1_dev);
>> +
>> +	mis_val = readl(rkisp1_dev->base_addr + CIF_MIPI_MIS);
>> +	if (mis_val)
>> +		rkisp1_mipi_isr(mis_val, rkisp1_dev);
>> +
>> +	mis_val = readl(rkisp1_dev->base_addr + CIF_MI_MIS);
>> +	if (mis_val)
>> +		rkisp1_mi_isr(mis_val, rkisp1_dev);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int rkisp1_plat_probe(struct platform_device *pdev)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +	const struct isp_match_data *clk_data;
>> +	const struct of_device_id *match;
>> +	struct device *dev = &pdev->dev;
>> +	struct rkisp1_device *isp_dev;
>> +	struct v4l2_device *v4l2_dev;
>> +	unsigned int i;
>> +	int ret, irq;
>> +
>> +	match = of_match_node(rkisp1_plat_of_match, node);
>> +	isp_dev = devm_kzalloc(dev, sizeof(*isp_dev), GFP_KERNEL);
>> +	if (!isp_dev)
>> +		return -ENOMEM;
>> +
>> +	INIT_LIST_HEAD(&isp_dev->sensors);
>> +
>> +	dev_set_drvdata(dev, isp_dev);
>> +	isp_dev->dev = dev;
>> +
>> +	isp_dev->base_addr = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(isp_dev->base_addr))
>> +		return PTR_ERR(isp_dev->base_addr);
>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +	if (irq < 0)
>> +		return irq;
>> +
>> +	ret = devm_request_irq(dev, irq, rkisp1_irq_handler, IRQF_SHARED,
>> +			       dev_driver_string(dev), dev);
>> +	if (ret < 0) {
>> +		dev_err(dev, "request irq failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	isp_dev->irq = irq;
>> +	clk_data = match->data;
>> +
>> +	for (i = 0; i < clk_data->size; i++)
>> +		isp_dev->clks[i].id = clk_data->clks[i];
>> +	ret = devm_clk_bulk_get(dev, clk_data->size, isp_dev->clks);
>> +	if (ret)
>> +		return ret;
>> +	isp_dev->clk_size = clk_data->size;
>> +
>> +	atomic_set(&isp_dev->pipe.power_cnt, 0);
>> +	atomic_set(&isp_dev->pipe.stream_cnt, 0);
>> +	isp_dev->pipe.open = rkisp1_pipeline_open;
>> +	isp_dev->pipe.close = rkisp1_pipeline_close;
>> +	isp_dev->pipe.set_stream = rkisp1_pipeline_set_stream;
>> +
>> +	rkisp1_stream_init(isp_dev, RKISP1_STREAM_SP);
>> +	rkisp1_stream_init(isp_dev, RKISP1_STREAM_MP);
>> +
>> +	strscpy(isp_dev->media_dev.model, "rkisp1",
>> +		sizeof(isp_dev->media_dev.model));
>> +	isp_dev->media_dev.dev = &pdev->dev;
>> +	strscpy(isp_dev->media_dev.bus_info,
>> +		"platform: " DRIVER_NAME, sizeof(isp_dev->media_dev.bus_info));
>> +	media_device_init(&isp_dev->media_dev);
>> +
>> +	v4l2_dev = &isp_dev->v4l2_dev;
>> +	v4l2_dev->mdev = &isp_dev->media_dev;
>> +	strscpy(v4l2_dev->name, "rkisp1", sizeof(v4l2_dev->name));
>> +	v4l2_ctrl_handler_init(&isp_dev->ctrl_handler, 5);
>> +	v4l2_dev->ctrl_handler = &isp_dev->ctrl_handler;
>> +
>> +	ret = v4l2_device_register(isp_dev->dev, &isp_dev->v4l2_dev);
>> +	if (ret < 0)
> 
> Once you've initialised the control handler, you'll need to free it in case
> of an error. I.e. add one more label for that purpose near the end.

control handler is not required, I'll remove it for the next version.

> 
>> +		return ret;
>> +
>> +	ret = media_device_register(&isp_dev->media_dev);
>> +	if (ret < 0) {
>> +		v4l2_err(v4l2_dev, "Failed to register media device: %d\n",
>> +			 ret);
>> +		goto err_unreg_v4l2_dev;
>> +	}
>> +
>> +	/* create & register platefom subdev (from of_node) */
>> +	ret = rkisp1_register_platform_subdevs(isp_dev);
>> +	if (ret < 0)
>> +		goto err_unreg_media_dev;
>> +
>> +	pm_runtime_enable(&pdev->dev);
>> +
>> +	return 0;
>> +
>> +err_unreg_media_dev:
>> +	media_device_unregister(&isp_dev->media_dev);
>> +err_unreg_v4l2_dev:
>> +	v4l2_device_unregister(&isp_dev->v4l2_dev);
>> +	return ret;
>> +}
>> +
>> +static int rkisp1_plat_remove(struct platform_device *pdev)
>> +{
>> +	struct rkisp1_device *isp_dev = platform_get_drvdata(pdev);
>> +
>> +	pm_runtime_disable(&pdev->dev);
>> +	media_device_unregister(&isp_dev->media_dev);
>> +	v4l2_async_notifier_unregister(&isp_dev->notifier);
>> +	v4l2_async_notifier_cleanup(&isp_dev->notifier);
>> +	v4l2_device_unregister(&isp_dev->v4l2_dev);
>> +	rkisp1_unregister_params_vdev(&isp_dev->params_vdev);
>> +	rkisp1_unregister_stats_vdev(&isp_dev->stats_vdev);
>> +	rkisp1_unregister_stream_vdevs(isp_dev);
>> +	rkisp1_unregister_isp_subdev(isp_dev);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __maybe_unused rkisp1_runtime_suspend(struct device *dev)
>> +{
>> +	struct rkisp1_device *isp_dev = dev_get_drvdata(dev);
>> +
>> +	clk_bulk_disable_unprepare(isp_dev->clk_size, isp_dev->clks);
>> +	return pinctrl_pm_select_sleep_state(dev);
>> +}
>> +
>> +static int __maybe_unused rkisp1_runtime_resume(struct device *dev)
>> +{
>> +	struct rkisp1_device *isp_dev = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = pinctrl_pm_select_default_state(dev);
>> +	if (ret < 0)
>> +		return ret;
>> +	ret = clk_bulk_prepare_enable(isp_dev->clk_size, isp_dev->clks);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct dev_pm_ops rkisp1_plat_pm_ops = {
>> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +				pm_runtime_force_resume)
>> +	SET_RUNTIME_PM_OPS(rkisp1_runtime_suspend, rkisp1_runtime_resume, NULL)
>> +};
>> +
>> +static struct platform_driver rkisp1_plat_drv = {
>> +	.driver = {
>> +		.name = DRIVER_NAME,
>> +		.of_match_table = of_match_ptr(rkisp1_plat_of_match),
>> +		.pm = &rkisp1_plat_pm_ops,
>> +	},
>> +	.probe = rkisp1_plat_probe,
>> +	.remove = rkisp1_plat_remove,
>> +};
>> +
>> +module_platform_driver(rkisp1_plat_drv);
>> +MODULE_AUTHOR("Rockchip Camera/ISP team");
>> +MODULE_DESCRIPTION("Rockchip ISP1 platform driver");
>> +MODULE_LICENSE("Dual BSD/GPL");
> 
> BSD or MIT?

Thanks for spotting this, I'll verify.

> 
>> diff --git a/drivers/media/platform/rockchip/isp1/dev.h b/drivers/media/platform/rockchip/isp1/dev.h
>> new file mode 100644
>> index 000000000000..f7cbee316523
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/isp1/dev.h
>> @@ -0,0 +1,97 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Rockchip isp1 driver
>> + *
>> + * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#ifndef _RKISP1_DEV_H
>> +#define _RKISP1_DEV_H
>> +
>> +#include <linux/clk.h>
>> +
>> +#include "capture.h"
>> +#include "rkisp1.h"
>> +#include "isp_params.h"
>> +#include "isp_stats.h"
>> +
>> +#define DRIVER_NAME "rkisp1"
>> +#define ISP_VDEV_NAME DRIVER_NAME  "_ispdev"
>> +#define SP_VDEV_NAME DRIVER_NAME   "_selfpath"
>> +#define MP_VDEV_NAME DRIVER_NAME   "_mainpath"
>> +#define DMA_VDEV_NAME DRIVER_NAME  "_dmapath"
>> +
>> +#define GRP_ID_SENSOR			BIT(0)
>> +#define GRP_ID_MIPIPHY			BIT(1)
>> +#define GRP_ID_ISP			BIT(2)
>> +#define GRP_ID_ISP_MP			BIT(3)
>> +#define GRP_ID_ISP_SP			BIT(4)
>> +
>> +#define RKISP1_MAX_BUS_CLK	8
>> +#define RKISP1_MAX_SENSOR	2
>> +#define RKISP1_MAX_PIPELINE	4
>> +
>> +/*
>> + * struct rkisp1_pipeline - An ISP hardware pipeline
>> + *
>> + * Capture device call other devices via pipeline
>> + *
>> + * @num_subdevs: number of linked subdevs
>> + * @power_cnt: pipeline power count
>> + * @stream_cnt: stream power count
>> + */
>> +struct rkisp1_pipeline {
>> +	struct media_pipeline pipe;
>> +	int num_subdevs;
>> +	atomic_t power_cnt;
>> +	atomic_t stream_cnt;
>> +	struct v4l2_subdev *subdevs[RKISP1_MAX_PIPELINE];
>> +	int (*open)(struct rkisp1_pipeline *p,
>> +		    struct media_entity *me, bool prepare);
>> +	int (*close)(struct rkisp1_pipeline *p);
>> +	int (*set_stream)(struct rkisp1_pipeline *p, bool on);
>> +};
>> +
>> +/*
>> + * struct rkisp1_sensor - Sensor information
>> + * @mbus: media bus configuration
>> + */
>> +struct rkisp1_sensor {
>> +	struct v4l2_subdev *sd;
>> +	struct v4l2_mbus_config mbus;
>> +	unsigned int lanes;
>> +	struct phy *dphy;
>> +	struct list_head list;
>> +};
> 
> You seem to also have struct sensor_async_subdev that appears to contain
> similar information. Would it be possible to unify the two?
> 
> This would appear to allow you getting rid of functions such as
> sd_to_sensor, for instance.

ack, I managed to get rid of this, and I don't even need to keep them
on a list, I'll submit in the next version.

Thanks a lot for your review
Helen

> 
>> +
>> +/*
>> + * struct rkisp1_device - ISP platform device
>> + * @base_addr: base register address
>> + * @active_sensor: sensor in-use, set when streaming on
>> + * @isp_sdev: ISP sub-device
>> + * @rkisp1_stream: capture video device
>> + * @stats_vdev: ISP statistics output device
>> + * @params_vdev: ISP input parameters device
>> + */
>> +struct rkisp1_device {
>> +	void __iomem *base_addr;
>> +	int irq;
>> +	struct device *dev;
>> +	unsigned int clk_size;
>> +	struct clk_bulk_data clks[RKISP1_MAX_BUS_CLK];
>> +	struct v4l2_device v4l2_dev;
>> +	struct v4l2_ctrl_handler ctrl_handler;
>> +	struct media_device media_dev;
>> +	struct v4l2_async_notifier notifier;
>> +	struct v4l2_subdev *subdevs[RKISP1_SD_MAX];
>> +	struct rkisp1_sensor *active_sensor;
>> +	struct list_head sensors;
>> +	struct rkisp1_isp_subdev isp_sdev;
>> +	struct rkisp1_stream stream[RKISP1_MAX_STREAM];
>> +	struct rkisp1_isp_stats_vdev stats_vdev;
>> +	struct rkisp1_isp_params_vdev params_vdev;
>> +	struct rkisp1_pipeline pipe;
>> +	struct vb2_alloc_ctx *alloc_ctx;
>> +};
>> +
>> +#endif
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 22/22] ARM: omap1: enable multiplatform
From: Arnd Bergmann @ 2019-08-08 21:47 UTC (permalink / raw)
  To: Tony Lindgren, Aaro Koskinen
  Cc: Arnd Bergmann, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
	Linus Walleij, linux-kernel, Tomi Valkeinen, linux-omap,
	linux-arm-kernel
In-Reply-To: <20190808212234.2213262-1-arnd@arndb.de>

With all the header files out of the way, and the clock driver
converted to drivers/clk/, nothing stops us from building
OMAP together with the other platforms.

As usual, the decompressor support is a victim here, and is
only available when CONFIG_DEBUG_LL is configured for the
particular board.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/Kconfig                              |  18 ---
 arch/arm/configs/omap1_defconfig              |   3 +
 arch/arm/mach-omap1/Kconfig                   |  20 ++-
 arch/arm/mach-omap1/Makefile                  |   4 +
 arch/arm/mach-omap1/hardware.h                |   2 +-
 arch/arm/mach-omap1/include/mach/uncompress.h | 117 ------------------
 arch/arm/mach-omap1/serial.c                  |   3 +-
 .../mach-omap1/{include/mach => }/serial.h    |   0
 arch/arm/plat-omap/Makefile                   |   1 +
 9 files changed, 26 insertions(+), 142 deletions(-)
 delete mode 100644 arch/arm/mach-omap1/include/mach/uncompress.h
 rename arch/arm/mach-omap1/{include/mach => }/serial.h (100%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 17a21f75f386..8542dfc5cf84 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -490,24 +490,6 @@ config ARCH_S3C24XX
 	  (<http://www.simtec.co.uk/products/EB110ITX/>), the IPAQ 1940 or the
 	  Samsung SMDK2410 development board (and derivatives).
 
-config ARCH_OMAP1
-	bool "TI OMAP1"
-	select ARCH_HAS_HOLES_MEMORYMODEL
-	select ARCH_OMAP
-	select CLKDEV_LOOKUP
-	select CLKSRC_MMIO
-	select COMMON_CLK
-	select FORCE_PCI if PCCARD
-	select GENERIC_CLOCKEVENTS
-	select GENERIC_IRQ_CHIP
-	select GENERIC_IRQ_MULTI_HANDLER
-	select GPIOLIB
-	select HAVE_IDE
-	select IRQ_DOMAIN
-	select SPARSE_IRQ
-	help
-	  Support for older TI OMAP1 (omap7xx, omap15xx or omap16xx)
-
 endchoice
 
 menu "Multiple platform selection"
diff --git a/arch/arm/configs/omap1_defconfig b/arch/arm/configs/omap1_defconfig
index 0c43c589f191..902125f89315 100644
--- a/arch/arm/configs/omap1_defconfig
+++ b/arch/arm/configs/omap1_defconfig
@@ -20,6 +20,9 @@ CONFIG_MODULE_FORCE_UNLOAD=y
 # CONFIG_BLK_DEV_BSG is not set
 # CONFIG_IOSCHED_DEADLINE is not set
 # CONFIG_IOSCHED_CFQ is not set
+CONFIG_ARCH_MULTI_V4T=y
+CONFIG_ARCH_MULTI_V5=y
+# CONFIG_ARCH_MULTI_V7 is not set
 CONFIG_ARCH_OMAP=y
 CONFIG_ARCH_OMAP1=y
 CONFIG_OMAP_RESET_CLOCKS=y
diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig
index 6a2c441ab579..5b1d3a24462f 100644
--- a/arch/arm/mach-omap1/Kconfig
+++ b/arch/arm/mach-omap1/Kconfig
@@ -1,4 +1,16 @@
 # SPDX-License-Identifier: GPL-2.0-only
+menuconfig ARCH_OMAP1
+	bool "TI OMAP1"
+	depends on ARCH_MULTI_V4T || ARCH_MULTI_V5
+	select ARCH_HAS_HOLES_MEMORYMODEL
+	select ARCH_OMAP
+	select CLKSRC_MMIO
+	select FORCE_PCI if PCCARD
+	select GPIOLIB
+	select HAVE_IDE
+	help
+	  Support for older TI OMAP1 (omap7xx, omap15xx or omap16xx)
+
 if ARCH_OMAP1
 
 menu "TI OMAP1 specific features"
@@ -14,27 +26,27 @@ config ARCH_OMAP1_AUTO
 	select MACH_OMAP_GENERIC if (ARCH_OMAP15XX || ARCH_OMAP16XX)
 
 config ARCH_OMAP730
-	depends on ARCH_OMAP1
+	depends on ARCH_MULTI_V5
 	bool "OMAP730 Based System"
 	select ARCH_OMAP_OTG
 	select CPU_ARM926T
 	select OMAP_MPU_TIMER
 
 config ARCH_OMAP850
-	depends on ARCH_OMAP1
+	depends on ARCH_MULTI_V5
 	bool "OMAP850 Based System"
 	select ARCH_OMAP_OTG
 	select CPU_ARM926T
 
 config ARCH_OMAP15XX
-	depends on ARCH_OMAP1
+	depends on ARCH_MULTI_V4T
 	default y
 	bool "OMAP15xx Based System"
 	select CPU_ARM925T
 	select OMAP_MPU_TIMER
 
 config ARCH_OMAP16XX
-	depends on ARCH_OMAP1
+	depends on ARCH_MULTI_V5
 	bool "OMAP16xx Based System"
 	select ARCH_OMAP_OTG
 	select CPU_ARM926T
diff --git a/arch/arm/mach-omap1/Makefile b/arch/arm/mach-omap1/Makefile
index 1337d7a2754c..1f0f97868953 100644
--- a/arch/arm/mach-omap1/Makefile
+++ b/arch/arm/mach-omap1/Makefile
@@ -3,6 +3,10 @@
 # Makefile for the linux kernel.
 #
 
+ccflags-$(CONFIG_ARCH_MULTIPLATFORM) += -I$(srctree)/arch/arm/mach-omap1/include
+asflags-$(CONFIG_ARCH_MULTIPLATFORM) += -I$(srctree)/arch/arm/mach-omap1/include
+ccflags-$(CONFIG_ARCH_MULTIPLATFORM) += -I$(srctree)/arch/arm/plat-omap/include
+
 # Common support
 obj-y := io.o id.o sram-init.o sram.o time.o irq.o mux.o flash.o \
 	 serial.o devices.o dma.o fb.o
diff --git a/arch/arm/mach-omap1/hardware.h b/arch/arm/mach-omap1/hardware.h
index 2cfc342c069c..232b8deef907 100644
--- a/arch/arm/mach-omap1/hardware.h
+++ b/arch/arm/mach-omap1/hardware.h
@@ -64,7 +64,7 @@ static inline u32 omap_cs3_phys(void)
 #define OMAP1_IO_OFFSET		0x01000000	/* Virtual IO = 0xfefb0000 */
 #define OMAP1_IO_ADDRESS(pa)	IOMEM((pa) - OMAP1_IO_OFFSET)
 
-#include <mach/serial.h>
+#include "serial.h"
 
 /*
  * ---------------------------------------------------------------------------
diff --git a/arch/arm/mach-omap1/include/mach/uncompress.h b/arch/arm/mach-omap1/include/mach/uncompress.h
deleted file mode 100644
index 9cca6a56788f..000000000000
--- a/arch/arm/mach-omap1/include/mach/uncompress.h
+++ /dev/null
@@ -1,117 +0,0 @@
-/*
- * arch/arm/plat-omap/include/mach/uncompress.h
- *
- * Serial port stubs for kernel decompress status messages
- *
- * Initially based on:
- * linux-2.4.15-rmk1-dsplinux1.6/arch/arm/plat-omap/include/mach1510/uncompress.h
- * Copyright (C) 2000 RidgeRun, Inc.
- * Author: Greg Lonnon <glonnon@ridgerun.com>
- *
- * Rewritten by:
- * Author: <source@mvista.com>
- * 2004 (c) MontaVista Software, Inc.
- *
- * This file is licensed under the terms of the GNU General Public License
- * version 2. This program is licensed "as is" without any warranty of any
- * kind, whether express or implied.
- */
-
-#include <linux/types.h>
-#include <linux/serial_reg.h>
-
-#include <asm/memory.h>
-#include <asm/mach-types.h>
-
-#include "serial.h"
-
-#define MDR1_MODE_MASK			0x07
-
-volatile u8 *uart_base;
-int uart_shift;
-
-/*
- * Store the DEBUG_LL uart number into memory.
- * See also debug-macro.S, and serial.c for related code.
- */
-static void set_omap_uart_info(unsigned char port)
-{
-	/*
-	 * Get address of some.bss variable and round it down
-	 * a la CONFIG_AUTO_ZRELADDR.
-	 */
-	u32 ram_start = (u32)&uart_shift & 0xf8000000;
-	u32 *uart_info = (u32 *)(ram_start + OMAP_UART_INFO_OFS);
-	*uart_info = port;
-}
-
-static inline void putc(int c)
-{
-	if (!uart_base)
-		return;
-
-	/* Check for UART 16x mode */
-	if ((uart_base[UART_OMAP_MDR1 << uart_shift] & MDR1_MODE_MASK) != 0)
-		return;
-
-	while (!(uart_base[UART_LSR << uart_shift] & UART_LSR_THRE))
-		barrier();
-	uart_base[UART_TX << uart_shift] = c;
-}
-
-static inline void flush(void)
-{
-}
-
-/*
- * Macros to configure UART1 and debug UART
- */
-#define _DEBUG_LL_ENTRY(mach, dbg_uart, dbg_shft, dbg_id)		\
-	if (machine_is_##mach()) {					\
-		uart_base = (volatile u8 *)(dbg_uart);			\
-		uart_shift = (dbg_shft);				\
-		port = (dbg_id);					\
-		set_omap_uart_info(port);				\
-		break;							\
-	}
-
-#define DEBUG_LL_OMAP7XX(p, mach)					\
-	_DEBUG_LL_ENTRY(mach, OMAP1_UART##p##_BASE, OMAP7XX_PORT_SHIFT,	\
-		OMAP1UART##p)
-
-#define DEBUG_LL_OMAP1(p, mach)						\
-	_DEBUG_LL_ENTRY(mach, OMAP1_UART##p##_BASE, OMAP_PORT_SHIFT,	\
-		OMAP1UART##p)
-
-static inline void arch_decomp_setup(void)
-{
-	int port = 0;
-
-	/*
-	 * Initialize the port based on the machine ID from the bootloader.
-	 * Note that we're using macros here instead of switch statement
-	 * as machine_is functions are optimized out for the boards that
-	 * are not selected.
-	 */
-	do {
-		/* omap7xx/8xx based boards using UART1 with shift 0 */
-		DEBUG_LL_OMAP7XX(1, herald);
-		DEBUG_LL_OMAP7XX(1, omap_perseus2);
-
-		/* omap15xx/16xx based boards using UART1 */
-		DEBUG_LL_OMAP1(1, ams_delta);
-		DEBUG_LL_OMAP1(1, nokia770);
-		DEBUG_LL_OMAP1(1, omap_h2);
-		DEBUG_LL_OMAP1(1, omap_h3);
-		DEBUG_LL_OMAP1(1, omap_innovator);
-		DEBUG_LL_OMAP1(1, omap_osk);
-		DEBUG_LL_OMAP1(1, omap_palmte);
-		DEBUG_LL_OMAP1(1, omap_palmz71);
-
-		/* omap15xx/16xx based boards using UART2 */
-		DEBUG_LL_OMAP1(2, omap_palmtt);
-
-		/* omap15xx/16xx based boards using UART3 */
-		DEBUG_LL_OMAP1(3, sx1);
-	} while (0);
-}
diff --git a/arch/arm/mach-omap1/serial.c b/arch/arm/mach-omap1/serial.c
index d6d1843337a5..a8db332dc72e 100644
--- a/arch/arm/mach-omap1/serial.c
+++ b/arch/arm/mach-omap1/serial.c
@@ -19,8 +19,7 @@
 
 #include <asm/mach-types.h>
 
-#include <mach/serial.h>
-
+#include "serial.h"
 #include "mux.h"
 #include "pm.h"
 #include "soc.h"
diff --git a/arch/arm/mach-omap1/include/mach/serial.h b/arch/arm/mach-omap1/serial.h
similarity index 100%
rename from arch/arm/mach-omap1/include/mach/serial.h
rename to arch/arm/mach-omap1/serial.h
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 371f2ed00eda..a11c96a093c9 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -4,6 +4,7 @@
 #
 
 ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/plat-omap/include
+ccflags-$(CONFIG_ARCH_OMAP1)	     += -I$(srctree)/arch/arm/mach-omap1/include
 
 # Common support
 obj-y := sram.o dma.o counter_32k.o
-- 
2.20.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related


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