| Message ID | 20210516014441.5508-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series | media: imx: imx7-mipi-csis: Add i.MX8MM support | expand |
On Sun, May 16, 2021 at 04:44:38AM +0300, Laurent Pinchart wrote: > Move the PHY regulator and reset handling to dedicated functions. This > groups all related code together, and prepares for i.MX8 support that > doesn't require control of the PHY regulator and reset. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> My apologies, this is a new patch in v2, and Rui hasn't acked it. I'll thus wait for acks and reviews before sending a pull request. > --- > drivers/staging/media/imx/imx7-mipi-csis.c | 64 +++++++++++++--------- > 1 file changed, 38 insertions(+), 26 deletions(-) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > index 6e235c86e0aa..a8da8d6ddb7d 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -457,25 +457,6 @@ static void mipi_csis_sw_reset(struct csi_state *state) > usleep_range(10, 20); > } > > -static int mipi_csis_phy_init(struct csi_state *state) > -{ > - state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > - if (IS_ERR(state->mipi_phy_regulator)) > - return PTR_ERR(state->mipi_phy_regulator); > - > - return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > - 1000000); > -} > - > -static void mipi_csis_phy_reset(struct csi_state *state) > -{ > - reset_control_assert(state->mrst); > - > - msleep(20); > - > - reset_control_deassert(state->mrst); > -} > - > static void mipi_csis_system_enable(struct csi_state *state, int on) > { > u32 val, mask; > @@ -679,6 +660,42 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +/* ----------------------------------------------------------------------------- > + * PHY regulator and reset > + */ > + > +static int mipi_csis_phy_enable(struct csi_state *state) > +{ > + return regulator_enable(state->mipi_phy_regulator); > +} > + > +static int mipi_csis_phy_disable(struct csi_state *state) > +{ > + return regulator_disable(state->mipi_phy_regulator); > +} > + > +static void mipi_csis_phy_reset(struct csi_state *state) > +{ > + reset_control_assert(state->mrst); > + msleep(20); > + reset_control_deassert(state->mrst); > +} > + > +static int mipi_csis_phy_init(struct csi_state *state) > +{ > + /* Get MIPI PHY reset and regulator. */ > + state->mrst = devm_reset_control_get_exclusive(state->dev, NULL); > + if (IS_ERR(state->mrst)) > + return PTR_ERR(state->mrst); > + > + state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > + if (IS_ERR(state->mipi_phy_regulator)) > + return PTR_ERR(state->mipi_phy_regulator); > + > + return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > + 1000000); > +} > + > /* ----------------------------------------------------------------------------- > * Debug > */ > @@ -1178,7 +1195,7 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime) > mutex_lock(&state->lock); > if (state->state & ST_POWERED) { > mipi_csis_stop_stream(state); > - ret = regulator_disable(state->mipi_phy_regulator); > + ret = mipi_csis_phy_disable(state); > if (ret) > goto unlock; > mipi_csis_clk_disable(state); > @@ -1204,7 +1221,7 @@ static int mipi_csis_pm_resume(struct device *dev, bool runtime) > goto unlock; > > if (!(state->state & ST_POWERED)) { > - ret = regulator_enable(state->mipi_phy_regulator); > + ret = mipi_csis_phy_enable(state); > if (ret) > goto unlock; > > @@ -1288,11 +1305,6 @@ static int mipi_csis_parse_dt(struct csi_state *state) > &state->clk_frequency)) > state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ; > > - /* Get MIPI PHY resets */ > - state->mrst = devm_reset_control_get_exclusive(state->dev, NULL); > - if (IS_ERR(state->mrst)) > - return PTR_ERR(state->mrst); > - > return 0; > } >
On Sun, May 16, 2021 at 04:44:16AM +0300, Laurent Pinchart wrote: > Hello, > > This patch series adds support for the CSIS found in the NXP i.MX8MM SoC > to the imx7-mipi-csis driver. > > The CSIS is an IP core from Samsung, integrated in different NXP SoCs. > The driver currently supports v3.3 of the CSIS, found in SoCs from the > i.MX6 and i.MX7 families. This series extends the driver to support > v3.6.3 of the IP, found in i.MX8MM and other members of the i.MX8 > family. > > The first 22 patches are miscellaneous cleanups and improvements. Please > see individual patches for details. > > Patch 23/25 extends the imx7-mipi-csis DT bindings with i.MX8MM support. > Support for other members of the i.MX8 family will come later, and for > SoCs including an ISI IP core (such as the i.MX8MP) this will require > more work to handle additional glue logic. > > Patch 24/25 then extends the imx7-mipi-csis driver accordingly. > > Finally, patch 25/25 updates MAINTAINERS per popular request from people > who believe I have too much free time :-) > > The series has been tested on an i.MX6ULL (for the CSIS v3.3) and I meant an i.MX7D, sorry. > i.MX8MM (for the CSIS v3.6.3). > > The changes in the integration of the CSIS between i.MX7 and i.MX8, as > described in the DT bindings, have been found through reading of > reference manuals and BSP source code, with different sources of > information contradicting each other. A confirmation from NXP would be > nice (in particular regarding the clocks). > > Laurent Pinchart (25): > media: imx: imx7_mipi_csis: Fix logging of only error event counters > media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts > media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel > mode > media: imx: imx7_mipi_csis: Move static data to top of > mipi_csis_dump_regs() > media: imx: imx7_mipi_csis: Minimize locking in get/set format > media: imx: imx7_mipi_csis: Don't set subdev data > media: imx: imx7_mipi_csis: Reorganize code in sections > media: imx: imx7_mipi_csis: Set the CLKSETTLE register field > media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure > media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure > media: imx: imx7_mipi_csis: Drop csi_state phy field > media: imx: imx7_mipi_csis: Rename mipi_sd to sd > media: imx: imx7_mipi_csis: Rename csi_state flag field to state > media: imx: imx7_mipi_csis: Turn csi_state irq field into local > variable > media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt() > media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init() > media: imx: imx7_mipi_csis: Drop csi_state pdev field > media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned > media: imx: imx7_mipi_csis: Reorganize csi_state structure > media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe() > media: imx: imx7_mipi_csis: Reject invalid data-lanes settings > media: imx: imx7_mipi_csis: Move PHY control to dedicated functions > dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support > media: imx: imx7_mipi_csis: Add i.MX8MM support > media: imx: imx7_mipi_csis: Update MAINTAINERS > > .../bindings/media/nxp,imx7-mipi-csi2.yaml | 109 +- > MAINTAINERS | 1 + > drivers/staging/media/imx/imx7-mipi-csis.c | 994 ++++++++++-------- > 3 files changed, 658 insertions(+), 446 deletions(-)
Hi Laurent, On Sun May 16, 2021 at 3:18 AM WEST, Laurent Pinchart wrote: > On Sun, May 16, 2021 at 04:44:38AM +0300, Laurent Pinchart wrote: > > Move the PHY regulator and reset handling to dedicated functions. This > > groups all related code together, and prepares for i.MX8 support that > > doesn't require control of the PHY regulator and reset. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> > > My apologies, this is a new patch in v2, and Rui hasn't acked it. I'll > thus wait for acks and reviews before sending a pull request. Looks good. Thanks for the heads up. Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> ------ Cheers, Rui > > > --- > > drivers/staging/media/imx/imx7-mipi-csis.c | 64 +++++++++++++--------- > > 1 file changed, 38 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > > index 6e235c86e0aa..a8da8d6ddb7d 100644 > > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > > @@ -457,25 +457,6 @@ static void mipi_csis_sw_reset(struct csi_state *state) > > usleep_range(10, 20); > > } > > > > -static int mipi_csis_phy_init(struct csi_state *state) > > -{ > > - state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > > - if (IS_ERR(state->mipi_phy_regulator)) > > - return PTR_ERR(state->mipi_phy_regulator); > > - > > - return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > > - 1000000); > > -} > > - > > -static void mipi_csis_phy_reset(struct csi_state *state) > > -{ > > - reset_control_assert(state->mrst); > > - > > - msleep(20); > > - > > - reset_control_deassert(state->mrst); > > -} > > - > > static void mipi_csis_system_enable(struct csi_state *state, int on) > > { > > u32 val, mask; > > @@ -679,6 +660,42 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > > return IRQ_HANDLED; > > } > > > > +/* ----------------------------------------------------------------------------- > > + * PHY regulator and reset > > + */ > > + > > +static int mipi_csis_phy_enable(struct csi_state *state) > > +{ > > + return regulator_enable(state->mipi_phy_regulator); > > +} > > + > > +static int mipi_csis_phy_disable(struct csi_state *state) > > +{ > > + return regulator_disable(state->mipi_phy_regulator); > > +} > > + > > +static void mipi_csis_phy_reset(struct csi_state *state) > > +{ > > + reset_control_assert(state->mrst); > > + msleep(20); > > + reset_control_deassert(state->mrst); > > +} > > + > > +static int mipi_csis_phy_init(struct csi_state *state) > > +{ > > + /* Get MIPI PHY reset and regulator. */ > > + state->mrst = devm_reset_control_get_exclusive(state->dev, NULL); > > + if (IS_ERR(state->mrst)) > > + return PTR_ERR(state->mrst); > > + > > + state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy"); > > + if (IS_ERR(state->mipi_phy_regulator)) > > + return PTR_ERR(state->mipi_phy_regulator); > > + > > + return regulator_set_voltage(state->mipi_phy_regulator, 1000000, > > + 1000000); > > +} > > + > > /* ----------------------------------------------------------------------------- > > * Debug > > */ > > @@ -1178,7 +1195,7 @@ static int mipi_csis_pm_suspend(struct device *dev, bool runtime) > > mutex_lock(&state->lock); > > if (state->state & ST_POWERED) { > > mipi_csis_stop_stream(state); > > - ret = regulator_disable(state->mipi_phy_regulator); > > + ret = mipi_csis_phy_disable(state); > > if (ret) > > goto unlock; > > mipi_csis_clk_disable(state); > > @@ -1204,7 +1221,7 @@ static int mipi_csis_pm_resume(struct device *dev, bool runtime) > > goto unlock; > > > > if (!(state->state & ST_POWERED)) { > > - ret = regulator_enable(state->mipi_phy_regulator); > > + ret = mipi_csis_phy_enable(state); > > if (ret) > > goto unlock; > > > > @@ -1288,11 +1305,6 @@ static int mipi_csis_parse_dt(struct csi_state *state) > > &state->clk_frequency)) > > state->clk_frequency = DEFAULT_SCLK_CSIS_FREQ; > > > > - /* Get MIPI PHY resets */ > > - state->mrst = devm_reset_control_get_exclusive(state->dev, NULL); > > - if (IS_ERR(state->mrst)) > > - return PTR_ERR(state->mrst); > > - > > return 0; > > } > > > > -- > Regards, > > Laurent Pinchart
On 16.05.21 03:44, Laurent Pinchart wrote: > Hello, > > This patch series adds support for the CSIS found in the NXP i.MX8MM SoC > to the imx7-mipi-csis driver. > > The CSIS is an IP core from Samsung, integrated in different NXP SoCs. > The driver currently supports v3.3 of the CSIS, found in SoCs from the > i.MX6 and i.MX7 families. This series extends the driver to support > v3.6.3 of the IP, found in i.MX8MM and other members of the i.MX8 > family. > > The first 22 patches are miscellaneous cleanups and improvements. Please > see individual patches for details. > > Patch 23/25 extends the imx7-mipi-csis DT bindings with i.MX8MM support. > Support for other members of the i.MX8 family will come later, and for > SoCs including an ISI IP core (such as the i.MX8MP) this will require > more work to handle additional glue logic. > > Patch 24/25 then extends the imx7-mipi-csis driver accordingly. > > Finally, patch 25/25 updates MAINTAINERS per popular request from people > who believe I have too much free time :-) > > The series has been tested on an i.MX6ULL (for the CSIS v3.3) and > i.MX8MM (for the CSIS v3.6.3). > > The changes in the integration of the CSIS between i.MX7 and i.MX8, as > described in the DT bindings, have been found through reading of > reference manuals and BSP source code, with different sources of > information contradicting each other. A confirmation from NXP would be > nice (in particular regarding the clocks). I already tested your pre-v1 off-list patches with my setup with ADV7280-M and I now tried it again with your v2 patchset and the RFC set for the bridge driver. It works fine with some additional changes specific to the data format as already mentioned here: [1]. Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> [1] https://patchwork.kernel.org/project/linux-media/cover/20210215042741.28850-1-laurent.pinchart@ideasonboard.com/#24139291 > > Laurent Pinchart (25): > media: imx: imx7_mipi_csis: Fix logging of only error event counters > media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts > media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel > mode > media: imx: imx7_mipi_csis: Move static data to top of > mipi_csis_dump_regs() > media: imx: imx7_mipi_csis: Minimize locking in get/set format > media: imx: imx7_mipi_csis: Don't set subdev data > media: imx: imx7_mipi_csis: Reorganize code in sections > media: imx: imx7_mipi_csis: Set the CLKSETTLE register field > media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure > media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure > media: imx: imx7_mipi_csis: Drop csi_state phy field > media: imx: imx7_mipi_csis: Rename mipi_sd to sd > media: imx: imx7_mipi_csis: Rename csi_state flag field to state > media: imx: imx7_mipi_csis: Turn csi_state irq field into local > variable > media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt() > media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init() > media: imx: imx7_mipi_csis: Drop csi_state pdev field > media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned > media: imx: imx7_mipi_csis: Reorganize csi_state structure > media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe() > media: imx: imx7_mipi_csis: Reject invalid data-lanes settings > media: imx: imx7_mipi_csis: Move PHY control to dedicated functions > dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support > media: imx: imx7_mipi_csis: Add i.MX8MM support > media: imx: imx7_mipi_csis: Update MAINTAINERS > > .../bindings/media/nxp,imx7-mipi-csi2.yaml | 109 +- > MAINTAINERS | 1 + > drivers/staging/media/imx/imx7-mipi-csis.c | 994 ++++++++++-------- > 3 files changed, 658 insertions(+), 446 deletions(-) >
Hello, This patch series adds support for the CSIS found in the NXP i.MX8MM SoC to the imx7-mipi-csis driver. The CSIS is an IP core from Samsung, integrated in different NXP SoCs. The driver currently supports v3.3 of the CSIS, found in SoCs from the i.MX6 and i.MX7 families. This series extends the driver to support v3.6.3 of the IP, found in i.MX8MM and other members of the i.MX8 family. The first 22 patches are miscellaneous cleanups and improvements. Please see individual patches for details. Patch 23/25 extends the imx7-mipi-csis DT bindings with i.MX8MM support. Support for other members of the i.MX8 family will come later, and for SoCs including an ISI IP core (such as the i.MX8MP) this will require more work to handle additional glue logic. Patch 24/25 then extends the imx7-mipi-csis driver accordingly. Finally, patch 25/25 updates MAINTAINERS per popular request from people who believe I have too much free time :-) The series has been tested on an i.MX6ULL (for the CSIS v3.3) and i.MX8MM (for the CSIS v3.6.3). The changes in the integration of the CSIS between i.MX7 and i.MX8, as described in the DT bindings, have been found through reading of reference manuals and BSP source code, with different sources of information contradicting each other. A confirmation from NXP would be nice (in particular regarding the clocks). Laurent Pinchart (25): media: imx: imx7_mipi_csis: Fix logging of only error event counters media: imx: imx7_mipi_csis: Count the CSI-2 debug interrupts media: imx: imx7_mipi_csis: Update ISP_CONFIG macros for quad pixel mode media: imx: imx7_mipi_csis: Move static data to top of mipi_csis_dump_regs() media: imx: imx7_mipi_csis: Minimize locking in get/set format media: imx: imx7_mipi_csis: Don't set subdev data media: imx: imx7_mipi_csis: Reorganize code in sections media: imx: imx7_mipi_csis: Set the CLKSETTLE register field media: imx: imx7_mipi_csis: Drop unused csis_hw_reset structure media: imx: imx7_mipi_csis: Store CSI-2 data type in format structure media: imx: imx7_mipi_csis: Drop csi_state phy field media: imx: imx7_mipi_csis: Rename mipi_sd to sd media: imx: imx7_mipi_csis: Rename csi_state flag field to state media: imx: imx7_mipi_csis: Turn csi_state irq field into local variable media: imx: imx7_mipi_csis: Don't pass pdev to mipi_csis_parse_dt() media: imx: imx7_mipi_csis: Pass csi_state to mipi_csis_subdev_init() media: imx: imx7_mipi_csis: Drop csi_state pdev field media: imx: imx7_mipi_csis: Make csi_state num_clocks field unsigned media: imx: imx7_mipi_csis: Reorganize csi_state structure media: imx: imx7_mipi_csis: Reorganize mipi_csis_probe() media: imx: imx7_mipi_csis: Reject invalid data-lanes settings media: imx: imx7_mipi_csis: Move PHY control to dedicated functions dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support media: imx: imx7_mipi_csis: Add i.MX8MM support media: imx: imx7_mipi_csis: Update MAINTAINERS .../bindings/media/nxp,imx7-mipi-csi2.yaml | 109 +- MAINTAINERS | 1 + drivers/staging/media/imx/imx7-mipi-csis.c | 994 ++++++++++-------- 3 files changed, 658 insertions(+), 446 deletions(-)