Re: [PATCH v2 26/34] staging: vchiq_arm: Set up dma ranges on child devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nicolas, Phil,
    I'm in the process of sending a v2 of this series, trying to
reduce the patch count by only picking what's required to support
the ISP.

This patch and [30/34] puzzles me a bit, so I'll ask a few questions.

On Mon, May 04, 2020 at 06:54:32PM +0200, Nicolas Saenz Julienne wrote:
> On Mon, 2020-05-04 at 12:26 +0300, Laurent Pinchart wrote:
> > From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> >
> > The VCHIQ driver now loads the audio, camera, codec, and vc-sm
> > drivers as platform drivers. However they were not being given
> > the correct DMA configuration.
> >
> > Call of_dma_configure with the parent (VCHIQ) parameters to be
> > inherited by the child.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
> >  .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c   | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index c5c7af28c1c8..15ccd624aaab 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -2733,6 +2733,12 @@ vchiq_register_child(struct platform_device *pdev,
> > const char *name)
> >  		child = NULL;
> >  	}
> >
> > +	/*
> > +	 * We want the dma-ranges etc to be copied from the parent VCHIQ device
> > +	 * to be passed on to the children too.
> > +	 */
> > +	of_dma_configure(&new_dev->dev, pdev->dev.of_node, true);
>
> I think you could use struct platform_device_info's of_node_reused. See patch
> 908f6fc3a1405 ('usb: musb: sunxi: propagate devicetree node to glue pdev') for
> an example. AFAIK of_dma_configure() is only to be used in bus code, and there
> has been a huge effort in the past to make sure it says as such. With a proper
> fwnode set of_dma_configure() will be called during the device's probe.

I just tried giving to the children devices the parent's fwnode and set
of_node_reused as done in 908f6fc3a1405 to avoid usage of
of_dma_configure, and now I have the parent being probed 4 times, one per
each children :) I guess that was not the expected result...

What is in your opinions the best course of actions for this and patch
[30/34] ? Can we live with 'of_dma_configure()' ? [1]

I am assuming currently the vchiq children are manually registered as
there are no entries in the DTS for them, so I'm kind of missing the
point of having [30/34] in. What am I missing ?

Thanks
  j

[1] There are a few things that are not clear to me though, possible due
to my rather poor understanding of this core parts. In particular, as
the dma-ranges are assigned to the parent 'soc' node, and I assume
propagated to the children such as the 'vchiq' node, isnt' setting the
'parent' field of each newly created platform device enough to have
the property propagated ?

>
> Regards,
> Nicolas
>


Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux