Re: [Linux-stm32] [PATCH v8 08/10] drm: stm: dw-mipi-dsi: let the bridge handle the HW version check

[Adrian Ratiu]

On Fri, 29 May 2020, Philippe CORNU <philippe.cornu@xxxxxx> wrote:
> Hi Adrian, and thank you very much for the patchset.  Thank you 
> also for having tested it on STM32F769 and STM32MP1.  Sorry for 
> the late response, Yannick and I will review it as soon as 
> possible and we will keep you posted.  Note: Do not hesitate to 
> put us in copy for the next version  (philippe.cornu@xxxxxx, 
> yannick.fertre@xxxxxx) Regards, Philippe :-) 

Hi Philippe,

Thank you very much for your previous and future STM testing, 
really appreciate it! I've CC'd Yannick until now but I'll also CC 
you sure :)

It's been over a month since I posted v8 and I was just gearing up 
to address all feedback, rebase & retest to prepare v9 but I'll 
wait a little longer, no problem, it's no rush.

Have an awesome day,
Adrian

> On 4/27/20 10:19 AM, Adrian Ratiu wrote:
> > The stm mipi-dsi platform driver added a version test in
> > commit fa6251a747b7 ("drm/stm: dsi: check hardware version")
> > so that HW revisions other than v1.3x get rejected. The rockchip
> > driver had no such check and just assumed register layouts are
> > v1.3x compatible.
> >
> > Having such tests was a good idea because only v130/v131 layouts
> > were supported at the time, however since adding multiple layout
> > support in the bridge, the version is automatically checked for
> > all drivers, compatible layouts get picked and unsupported HW is
> > automatically rejected by the bridge, so there's no use keeping
> > the test in the stm driver.
> >
> > The main reason prompting this change is that the stm driver
> > test immediately disabled the peripheral clock after reading
> > the version, making the bridge read version 0x0 immediately
> > after in its own probe(), so we move the clock disabling after
> > the bridge does the version test.
> >
> > Tested on STM32F769 and STM32MP1.
> >
> > Cc: linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx
> > Reported-by: Adrian Pop <pop.adrian61@xxxxxxxxx>
> > Tested-by: Adrian Pop <pop.adrian61@xxxxxxxxx>
> > Tested-by: Arnaud Ferraris <arnaud.ferraris@xxxxxxxxxxxxx>
> > Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx>
> > ---
> > New in v6.
> > ---
> >   drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 12 +++---------
> >   1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > index 2e1f2664495d0..7218e405d7e2b 100644
> > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> > @@ -402,15 +402,6 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
> >   		goto err_dsi_probe;
> >   	}
> >   
> > -	dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> > -	clk_disable_unprepare(pclk);
> > -
> > -	if (dsi->hw_version != HWVER_130 && dsi->hw_version != HWVER_131) {
> > -		ret = -ENODEV;
> > -		DRM_ERROR("bad dsi hardware version\n");
> > -		goto err_dsi_probe;
> > -	}
> > -
> >   	dw_mipi_dsi_stm_plat_data.base = dsi->base;
> >   	dw_mipi_dsi_stm_plat_data.priv_data = dsi;
> >   
> > @@ -423,6 +414,9 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
> >   		goto err_dsi_probe;
> >   	}
> >   
> > +	dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> > +	clk_disable_unprepare(pclk);
> > +
> >   	return 0;
> >   
> >   err_dsi_probe: