Hi Sam
On Sat, Oct 16, 2021 at 2:25 PM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote:
>
> Hi Michael,
>
> I fail to follow the logic in this patch.
>
>
> On Sat, Oct 16, 2021 at 10:22:32AM +0000, Michael Trimarchi wrote:
> > The dsi registration is implemented in rockchip platform driver.
> > The attach can be called before the probe is terminated and therefore
> > we need to be sure that corresponding entry during attach is valid
> >
> > Signed-off-by: Michael Trimarchi <michael@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++++++-
> > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++++++----
> > include/drm/bridge/dw_mipi_dsi.h | 2 +-
> > 3 files changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index e44e18a0112a..44b211be15fc 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> > dsi->device_found = true;
> > }
> >
> > + /*
> > + * NOTE: the dsi registration is implemented in
> > + * platform driver, that to say dsi would be exist after
> > + * probe is terminated. The call is done before the end of probe
> > + * so we need to pass the dsi to the platform driver.
> > + */
> > if (pdata->host_ops && pdata->host_ops->attach) {
> > - ret = pdata->host_ops->attach(pdata->priv_data, device);
> > + ret = pdata->host_ops->attach(pdata->priv_data, device, dsi);
> > if (ret < 0)
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > index a2262bee5aa4..32ddc8642ec1 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > @@ -997,7 +997,8 @@ static const struct component_ops dw_mipi_dsi_rockchip_ops = {
> > };
> >
> > static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
> > - struct mipi_dsi_device *device)
> > + struct mipi_dsi_device *device,
> > + struct dw_mipi_dsi *dmd)
> > {
> > struct dw_mipi_dsi_rockchip *dsi = priv_data;
> > struct device *second;
> > @@ -1005,6 +1006,8 @@ static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
> >
> > mutex_lock(&dsi->usage_mutex);
> >
> > + dsi->dmd = dmd;
> > +
> > if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> > DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> > mutex_unlock(&dsi->usage_mutex);
> > @@ -1280,6 +1283,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > struct device_node *np = dev->of_node;
> > + struct dw_mipi_dsi *dmd;
> > struct dw_mipi_dsi_rockchip *dsi;
> > struct phy_provider *phy_provider;
> > struct resource *res;
> > @@ -1391,9 +1395,9 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
> > if (IS_ERR(phy_provider))
> > return PTR_ERR(phy_provider);
> >
> > - dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
> > - if (IS_ERR(dsi->dmd)) {
> > - ret = PTR_ERR(dsi->dmd);
> > + dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
> > + if (IS_ERR(dmd)) {
> > + ret = PTR_ERR(dmd);
>
> The memory pointed to by dmd is allocated in dw_mipi_dsi_probe(), but
> the pointer is not saved here.
> We rely on the attach operation to save the dmd pointer.
>
>
> In other words - the attach operation must be called before we call
> dw_mipi_dsi_rockchip_remove(), which uses the dmd member.
>
> This all looks wrong to me - are we papering over some other issue
Ok, it's wrong. I was not expecting that call.Anyway this was my path
on linux-next
dw_mipi_dsi_rockchip_probe
dw_mipi_dsi_probe -->start call
dw_mipi_dsi_rockchip_host_attach <-- this was not able to use dmd
dw_mipi_dsi_probe -> exit from the call
Michael
> here?
>
> Sam
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@xxxxxxxxxxxxxxxxxxxx
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@xxxxxxxxxxxxxxxxxxxx
www.amarulasolutions.com
