| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <25ed971e90324ad596353ba6aade2e14c40c7183.camel@puri.sm>
Date: Sun, 12 Mar 2023 15:04:09 +0100
From: Martin Kepplinger <martin.kepplinger@...i.sm>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: slongerbeam@...il.com, p.zabel@...gutronix.de, mchehab@...nel.org,
gregkh@...uxfoundation.org, shawnguo@...nel.org,
kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
kernel@...i.sm
Subject: Re: [PATCH v2 2/2] media: imx: imx8mq-mipi-csi2: remove unneeded
state variable and function
Am Sonntag, dem 12.03.2023 um 15:37 +0200 schrieb Laurent Pinchart:
> Hi Martin,
>
> Thank you for the patch.
>
> On Tue, Mar 07, 2023 at 04:00:47PM +0100, Martin Kepplinger wrote:
> > Clean up the driver a bit by inlining the
> > imx8mq_mipi_csi_system_enable()
> > function to the callsites and removing the hs_settle variable from
> > the
> > driver state.
> >
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@...i.sm>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>
> Could I volunteer you to also drop the struct csi_state state field ?
> :-)
sure :) it can become at least a bit more tricky than this patch. I'll
take the time after this is merged.
thanks for the fast reviewing
>
> > ---
> > drivers/staging/media/imx/imx8mq-mipi-csi2.c | 41 ++++++++--------
> > ----
> > 1 file changed, 16 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > index 1aa8622a3bae..f10b59b8f1c0 100644
> > --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c
> > @@ -119,9 +119,8 @@ struct csi_state {
> >
> > struct v4l2_mbus_config_mipi_csi2 bus;
> >
> > - struct mutex lock; /* Protect state and hs_settle */
> > + struct mutex lock; /* Protect state */
> > u32 state;
> > - u32 hs_settle;
> >
> > struct regmap *phy_gpr;
> > u8 phy_gpr_reg;
> > @@ -264,23 +263,6 @@ static int imx8mq_mipi_csi_sw_reset(struct
> > csi_state *state)
> > return 0;
> > }
> >
> > -static void imx8mq_mipi_csi_system_enable(struct csi_state *state,
> > int on)
> > -{
> > - if (!on) {
> > - imx8mq_mipi_csi_write(state,
> > CSI2RX_CFG_DISABLE_DATA_LANES, 0xf);
> > - return;
> > - }
> > -
> > - regmap_update_bits(state->phy_gpr,
> > - state->phy_gpr_reg,
> > - 0x3fff,
> > - GPR_CSI2_1_RX_ENABLE |
> > - GPR_CSI2_1_VID_INTFC_ENB |
> > - GPR_CSI2_1_HSEL |
> > - GPR_CSI2_1_CONT_CLK_MODE |
> > - GPR_CSI2_1_S_PRG_RXHS_SETTLE(state-
> > >hs_settle));
> > -}
> > -
> > static void imx8mq_mipi_csi_set_params(struct csi_state *state)
> > {
> > int lanes = state->bus.num_data_lanes;
> > @@ -321,7 +303,8 @@ static int imx8mq_mipi_csi_clk_get(struct
> > csi_state *state)
> > }
> >
> > static int imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> > - struct v4l2_subdev_state
> > *sd_state)
> > + struct v4l2_subdev_state
> > *sd_state,
> > + u32 *hs_settle)
> > {
> > s64 link_freq;
> > u32 lane_rate;
> > @@ -377,10 +360,10 @@ static int
> > imx8mq_mipi_csi_calc_hs_settle(struct csi_state *state,
> > max_ths_settle = 140 + 10 * 1000000 / (lane_rate / 1000);
> > ths_settle_ns = (min_ths_settle + max_ths_settle) / 2;
> >
> > - state->hs_settle = ths_settle_ns / esc_clk_period_ns - 1;
> > + *hs_settle = ths_settle_ns / esc_clk_period_ns - 1;
> >
> > dev_dbg(state->dev, "lane rate %u Ths_settle %u hs_settle
> > %u\n",
> > - lane_rate, ths_settle_ns, state->hs_settle);
> > + lane_rate, ths_settle_ns, *hs_settle);
> >
> > return 0;
> > }
> > @@ -389,24 +372,32 @@ static int
> > imx8mq_mipi_csi_start_stream(struct csi_state *state,
> > struct v4l2_subdev_state
> > *sd_state)
> > {
> > int ret;
> > + u32 hs_settle;
> >
> > ret = imx8mq_mipi_csi_sw_reset(state);
> > if (ret)
> > return ret;
> >
> > imx8mq_mipi_csi_set_params(state);
> > - ret = imx8mq_mipi_csi_calc_hs_settle(state, sd_state);
> > + ret = imx8mq_mipi_csi_calc_hs_settle(state, sd_state,
> > &hs_settle);
> > if (ret)
> > return ret;
> >
> > - imx8mq_mipi_csi_system_enable(state, true);
> > + regmap_update_bits(state->phy_gpr,
> > + state->phy_gpr_reg,
> > + 0x3fff,
> > + GPR_CSI2_1_RX_ENABLE |
> > + GPR_CSI2_1_VID_INTFC_ENB |
> > + GPR_CSI2_1_HSEL |
> > + GPR_CSI2_1_CONT_CLK_MODE |
> > +
> > GPR_CSI2_1_S_PRG_RXHS_SETTLE(hs_settle));
> >
> > return 0;
> > }
> >
> > static void imx8mq_mipi_csi_stop_stream(struct csi_state *state)
> > {
> > - imx8mq_mipi_csi_system_enable(state, false);
> > + imx8mq_mipi_csi_write(state, CSI2RX_CFG_DISABLE_DATA_LANES,
> > 0xf);
> > }
> >
> > /* ---------------------------------------------------------------
> > --------------
>
Powered by blists - more mailing lists