Date: Wed, 26 Jun 2024 15:07:11 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Peng Fan <peng.fan@....com>
Cc: Cristian Marussi <cristian.marussi@....com>,
	"Peng Fan (OSS)" <peng.fan@....nxp.com>,
	Sudeep Holla <sudeep.holla@....com>,
	"arm-scmi@...r.kernel.org" <arm-scmi@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/2] firmware: arm_scmi: create scmi devices for
 protocols that not have of_node

On Wed, Jun 26, 2024 at 11:50:26AM +0000, Peng Fan wrote:
> Hi Cristian,
> 
> > Subject: Re: [PATCH 0/2] firmware: arm_scmi: create scmi devices for
> > protocols that not have of_node
> > 
> > On Wed, Jun 26, 2024 at 02:58:38PM +0800, Peng Fan (OSS) wrote:
> > > Per
> > 
> > Hi,
> > 
> > >
> 
> ...
> > >
> > > i.MX95 SCMI firmware not have dedicated channel for 0x12, and no
> > need
> > > of_node. This patchset is to support protocol 0x12 without the
> > > procotol node in device tree.
> > >
> > 
> > With this patch you change a bit of the core logic to allow for protocols
> > not explicitly described in the DT to be instantiated, and you use a
> > static builtin array to list such protocols...so any future change or any
> > downstream vendor protocols that want to use this, we will have to
> > patch and extend such protocols[] array.
> > 
> > Moreover, if anyone DO want to use a per-protocol channel in the
> > future on some of these protocols, it will work fine with your solution
> > on the code side, BUT you will still have anyway a DT binding check
> > error when you try to add that 0x12 node to contain a channel
> > description, right ?
> 
> Right.
> 
> > ... because in that case you will have re-added a (supposedly) empty
> > protocol node in order to containn the channels definitions and that
> > wont be yaml-compliant, am I right ?
> > 
> > IOW this solves your issue in the immediate, while adding complexity
> > to the core code and changing the core behaviour around protocols,
> > but it wont stand any future addition or different usage.
> > 
> > For these reasons, I still think that the cleanest solution is to just let
> > protocol nodes to exist even if not referenced anywhere from the DT
> > (your original patch to add protocol0x12 I think) simply because we
> > allow per-protocol channel definitions and so any empty unreferenced
> > protocol node could be needed in the future for this reason.
> 
> You mean this one [1], right?
> 
> I could rebase and send out it again.
> 
> > 
> > In this way we'll also keep treating protocols in an uniform way.
> > 
> > Just my opinion, though, I'll settle with what is finally decided anyway.
> 
> From reading the previous discussion as listed in cover letter,
> I thought there was an agreement that for non consumers, no per
> protocol channel node, we should not add it in device tree.
> But indeed binding is needed in case the channel has its own channel.
> 
> This patchset could be dropped if Sudeep and you both agree with [1]
>  

Yes indeed, not sure what at the end Sudeep thinks about this after
reading that thread again....that's why I specified that was just my opinion :P

Moreover, regarding this series, I wonder if, in general, allowing
protocol devices without an underlyng DT node could not be asking for
trouble in the future...in the sense that these devices are used by SCMI
drivers and can be used by them as they wish, including using them to
register with other subsytems, subsystems that can have assumnptions on
the fact the device has a valid underlying of_node... I maybe overthinking...

Anyway let's see if Sudeep prefers to go down this way I will post some
more comments on specifically how this series works..

Thanks,
Cristian

Powered by blists - more mailing lists