Re: [PATCH 2/6] ASoC: meson: add audin main module and I2S formatter

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Valerio Setti <vsetti@baylibre.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	linux-sound@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/6] ASoC: meson: add audin main module and I2S formatter
Date: Mon, 14 Sep 2026 17:45:43 +0200	[thread overview]
Message-ID: <f2186082-8799-4eb6-8e0e-2b2036578661@kernel.org> (raw)
In-Reply-To: <964d6626-202c-4d0a-aef2-86a383f405ab@baylibre.com>

On 14/09/2026 15:14, Valerio Setti wrote:
> Thanks for your review and feedbacks.
> 
> 
> On 9/13/26 11:41 AM, Krzysztof Kozlowski wrote:
>> On Sat, Sep 12, 2026 at 12:14:32AM +0200, Valerio Setti wrote:
>>> +static int meson_gx_audin_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct resource *res;
>>> +	void __iomem *mmio;
>>> +	struct regmap *regmap;
>>> +	struct clk *clk;
>>> +	int ret;
>>> +
>>> +	ret = device_reset(dev);
>>> +	if (ret)
>>> +		return dev_err_probe(dev, ret, "Failed to reset device\n");
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	if (!res)
>>> +		return dev_err_probe(dev, -EINVAL, "Failed to get memory resource\n");
>>> +
>>> +	/*
>>> +	 * Do not use devm_platform_ioremap_resource() here: it would claim the
>>> +	 * whole AUDIN window exclusively and the FIFO children would then fail
>>> +	 * to request their own sub-ranges.
>>> +	 */
>>
>> And this should point you that child is not a separate device.
>>
>> Please open your datasheet and read what is the address space of this
>> device. You claim here ENTIRE address space. If you insist that entire
>> address space is not owned by this device, then you misrepresented
>> hardware in the DT anyway.
> 
> As I mentioned in the cover letter I designed it this way so that I 
> could easily re-use the same driver code for all 3 FIFOs just offsetting 
> the base address. I knew it was a bit hackish a solution, but IMO that 
> was worth it since it was simplifying a bit driver's code. Perhaps it 
> was more software-oriented rather than hardware related.
> The other possible approach is to fully copy what AIU is doing and 
> allocate each FIFO as a DAI. I'll follow that design then, but if you 
> have any better idea please let me know.

Do not design hardware description - so DTS - to facilitate your driver
design. Specifically requested by writing bindings.

Best regards,
Krzysztof

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic