Re: [PATCH] firmware: arm_sdei: remove unused interfaces

From: James Morse
Date: Tue May 19 2020 - 11:47:34 EST


Hi Xiongfeng,

On 05/05/2020 03:08, Xiongfeng Wang wrote:
> On 2020/5/5 1:14, James Morse wrote:
>> Hi Christoph,
>>
>> (CC: +Xiongfeng)
>>
>> Thanks for the reminder - I was just searching for who was using this.
>
> Thanks for CC me. We do have a driver that are using it.

>> On 04/05/2020 17:42, Christoph Hellwig wrote:
>>> The export symbols to register/unregister and enable/disable events
>>> aren't ever used outside of arm_sdei.c, so mark them static.
>>
>> Xiongfeng, you have drivers using this, could they be posted upstream. Or can we stop
>> exporting these?
>
> It's the SDEI Wathchdog which is used for hardlockup detection. But I wasn't
> able to push it upstream because we have Pseudo-NMI in mainline.

Hmm, that shouldn't be directly relevant, unless your SDEI watchdog is using the
bindable-irq thing?


If your firmware offers an event-id for the watchdog, please upstream the driver. Half of
the event-id space is reserved for vendor stuff like this.

If firmware needs to be told to re-configure the watchdog irq to make this work, then pNMI
is a much better fit. Having firmware and linux modifying the irqchip hardware is a
nightmare best avoided.


>> (they were originally added for the GHES RAS thing, but by the time it was all merged
>> upstream, it wasn't possible to build it as a module)
>
> The SDEI Watchdog driver also can't be built as a module. We still need to
> modify the origin kernel. So I think this patch doesn't affect me. Thanks for CC me.

Okay, I'll pick this up to drop the module exports.

I'd prefer not to make all this static as these register/unregister calls are the
interface that is supposed to be used. If we are going to gut it, we should do it completely.


Thanks,

James