lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e3caf226-2133-c570-b775-17ed3c7fbd02@i2se.com>
Date:   Mon, 1 Nov 2021 19:58:00 +0100
From:   Stefan Wahren <stefan.wahren@...e.com>
To:     Ojaswin Mujoo <ojaswin98@...il.com>
Cc:     nsaenz@...nel.org, gregkh@...uxfoundation.org,
        dan.carpenter@...cle.com, phil@...pberrypi.com,
        linux-arm-kernel@...ts.infradead.org,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] staging: vchiq: Replace global state with per device
 state

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@...il.com>
>> ...
>>>  
>>>  /*
>>> @@ -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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ