Re: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop

From: "sebastian.fricke@collabora.com" <sebastian.fricke@collabora.com>
To: Nicolas Dufresne <nicolas@ndufresne.ca>
Cc: Nas Chung <nas.chung@chipsnmedia.com>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"jackson.lee" <jackson.lee@chipsnmedia.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Subject: Re: [PATCH v2 4/5] media: chips-media: wave5: drop "sram-size" DT prop
Date: Fri, 5 Apr 2024 10:56:04 +0200	[thread overview]
Message-ID: <20240405085604.lzsekrtrtudvdh6c@basti-XPS-13-9310> (raw)
In-Reply-To: <4f781313ddb2b7657fadc35dc04c0e3c3de3b27c.camel@ndufresne.ca>

Hey Nicolas,

On 04.04.2024 14:56, Nicolas Dufresne wrote:
>Hi,
>
>Le jeudi 04 avril 2024 à 10:02 +0200, sebastian.fricke@collabora.com a écrit :
>> > > > > > Wave5 can enable/disable the sec_axi_info option for each instance.
>> > > > > >
>> > > > > > How about handle sram-size through match_data ?
>> > > > > > I can find some drivers which use match_data to configure the sram
>> > > size.
>> >
>> > I proposed using match_data to allocate different sram size for each device.
>> > Do you think this is a reasonable approach ?
>>
>> As discussed here:
>> https://patchwork.linuxtv.org/project/linux-media/patch/20240201184238.2542695-1-b-brnich@ti.com/
>>
>> To quote Brandon Brnich from TI:
>>
>> If static SRAM allocation is the correct method to go, then this series
>> can be ignored and I will add section in device tree and remove check
>> for parameter in driver as that will now be a bug.
>>
>> I would like to adhere to that.
>
>I feel your statement is a bit ambiguous. Can you clarify what you adhere too ?
>That we should hardcode fixed sram size in the DT ?

Sorry wasn't aware that my statement is ambiguous, my intention was to
say that we do not add the sram size to the DT as it was discussed with
the DT maintainer in the thread above, my adherence was pointed towards
the statement from Brandon: "then this series can be ignored".

>
>When I read earlier today "How about handle sram-size through match_data ?",
>what I saw was a static C structure in the driver. Like what we do with HW
>variants usually.
>
>I was pretty convince that the right solution is to allocate sram when the
>driver is used (open() seems appropriate), and drop it when its not used. With
>the clear benefit that we can change our mind later if we find valid arguments.
>
>So with that in mind, dropping (cleaning up) that old code seems helpful to
>create a clean slate to re-introduce a better version.
>
>Nicolas

Greetings,
Sebastian