On Mon, Nov 01, 2021 at 01:19:17PM +0100, Stefan Wahren wrote: > Hi Ojaswin, > > Am 01.11.21 um 12:09 schrieb Ojaswin Mujoo: > > Currently, the driver has a global g_state variable which is initialised > > during probe and directly used all over the driver code. However, this > > prevents the driver to support multiple VideoCore VPUs at the same time. > > > > Replace this global state with a per device state which is initialised > > and allocated during probing. > > > > Signed-off-by: Ojaswin Mujoo <ojaswin98@xxxxxxxxx> > ... > > > > /* > > @@ -1763,6 +1795,7 @@ static int vchiq_probe(struct platform_device *pdev) > > struct device_node *fw_node; > > const struct of_device_id *of_id; > > struct vchiq_drvdata *drvdata; > > + struct vchiq_device *vchiq_dev; > > int err; > > > > of_id = of_match_node(vchiq_of_match, pdev->dev.of_node); > > @@ -1784,7 +1817,18 @@ static int vchiq_probe(struct platform_device *pdev) > > > > platform_set_drvdata(pdev, drvdata); > > > > - err = vchiq_platform_init(pdev, &g_state); > > + vchiq_dev = kzalloc(sizeof(struct vchiq_device), GFP_KERNEL); > > + vchiq_dev->state = kzalloc(sizeof(struct vchiq_state), GFP_KERNEL); > > + vchiq_dev->vchiq_pdev = *pdev; > > + > > + g_state = vchiq_dev->state; > > + > > just a quick idea: how about storing the global state within vchiq_drvdata? > > So there is no need to reinvent somekind of vchiq device which is the > "same" as the platform device. After that you are able to access the > private driver data via platform_get_drvdata(). Hi Stefan, Thank for looking into this. I agree that the vchiq_device is just a sorta extension of the pdev. However, regarding using the drvdata, I had some questions. So I understand the purpose of this patch is to make sure our driver is able to support multiple devices, that is it can work with say, an SoC with 2 VideoCore VPUs. In this case, we would need to maintain 2 states for each VPU instead of a global state, however if we use something like the following: drvdata = (struct vchiq_drvdata *)of_id->data; drvdata->state = kzalloc(sizeof(struct vchiq_state), GFP_KERNEL); platform_set_drvdata(pdev, drvdata); Correct me if I'm wrong but I think the assignment to drvdata in line 1 above will return a pointer to the same structure. In which case, the line 2 will always overwrite the older state that is present. That's why I was initialising a separate object (vchiq_device) everytime the probe was called so that each device can have their own device specific structs. Please let me know if my understanding of drvdata is wrong here. > > Best regards > Thank you! Ojaswin
