Hi Ojawin, Am 01.11.21 um 18:31 schrieb Ojaswin Mujoo: > 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); > no, this wasn't my intention. We need a new container struct (maybe vchiq_priv or so) which stores the g_state and the platform data from the devicetree of_id->data. At first the probe function allocates this struct and then we fill it with g_state and the devicetree stuff. After that we attach it to the platform device. Regards
