linux-kernel - Re: [PATCH 3/7] drivers: soc: xilinx: add check for platform

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 2 Oct 2023 08:09:50 +0200
From:   Michal Simek <michal.simek@....com>
To:     Greg KH <gregkh@...uxfoundation.org>,
        Jay Buddhabhatti <jay.buddhabhatti@....com>
CC:     <tanmay.shah@....com>, <sai.krishna.potthuri@....com>,
        <nava.kishore.manne@....com>, <ben.levinsky@....com>,
        <dhaval.r.shah@....com>, <marex@...x.de>, <robh@...nel.org>,
        <arnd@...db.de>, <izhar.ameer.shaikh@....com>,
        <ruanjinjie@...wei.com>, <mathieu.poirier@...aro.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 3/7] drivers: soc: xilinx: add check for platform



On 9/30/23 09:17, Greg KH wrote:
> On Fri, Sep 29, 2023 at 03:55:54AM -0700, Jay Buddhabhatti wrote:
>> Some error event IDs for Versal and Versal NET are different.
>> Both the platforms should access their respective error event
>> IDs so use sub_family_code to check for platform and check
>> error IDs for respective platforms. The family code is passed
>> via platform data to avoid platform detection again.
>> Platform data is setup when even driver is registered.
>>
>> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@....com>
>> ---
>>   drivers/firmware/xilinx/zynqmp.c        |  2 +-
>>   drivers/soc/xilinx/xlnx_event_manager.c | 25 ++++++++++++++++++++-----
>>   include/linux/firmware/xlnx-zynqmp.h    | 16 ++++++++++++----
>>   3 files changed, 33 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
>> index 5f40288f69a9..6583efa9ac48 100644
>> --- a/drivers/firmware/xilinx/zynqmp.c
>> +++ b/drivers/firmware/xilinx/zynqmp.c
>> @@ -2019,7 +2019,7 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
>>   	np = of_find_compatible_node(NULL, NULL, "xlnx,versal");
>>   	if (np) {
>>   		em_dev = platform_device_register_data(&pdev->dev, "xlnx_event_manager",
>> -						       -1, NULL, 0);
>> +						       -1, &pm_sub_family_code, 4);
>>   		if (IS_ERR(em_dev))
>>   			dev_err_probe(&pdev->dev, PTR_ERR(em_dev), "EM register fail with error\n");
>>   	}
>> diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c
>> index 38cfc161a713..8074ded7b39c 100644
>> --- a/drivers/soc/xilinx/xlnx_event_manager.c
>> +++ b/drivers/soc/xilinx/xlnx_event_manager.c
>> @@ -23,6 +23,7 @@ static DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number1);
>>   
>>   static int virq_sgi;
>>   static int event_manager_availability = -EACCES;
>> +static u32 pm_sub_family_code;
> 
> Why is this not a per-device variable?  Global variables like this for
> drivers are almost always wrong.

Not sure I get what you mean by per-device. Can you please elaborate on it?
This is the part of firmware infrastructure and there is only one instance of 
the driver in the system registered from firmware driver itself.

If this should be done in a generic way then there are much more variables which 
should be moved to driver data not just this one.

Thanks,
Michal