[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <47bd1689-292f-4d2d-a668-6da81db761e5@altera.com>
Date: Mon, 15 Dec 2025 23:53:26 +0000
From: "Romli, Khairul Anuar" <khairul.anuar.romli@...era.com>
To: Rob Herring <robh@...nel.org>
CC: Dinh Nguyen <dinguyen@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Eugeniy Paltsev
<Eugeniy.Paltsev@...opsys.com>, Vinod Koul <vkoul@...nel.org>,
"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] dma: dw-axi-dmac: Add support for Agilex5 and
dynamic bus width
On 12/12/2025 9:19 pm, Rob Herring wrote:
> On Thu, Dec 11, 2025 at 6:46 PM Romli, Khairul Anuar
> <khairul.anuar.romli@...era.com> wrote:
>>
>> On 11/12/2025 11:52 pm, Rob Herring wrote:
>>> On Thu, Dec 11, 2025 at 9:45 AM Rob Herring <robh@...nel.org> wrote:
>>>>
>>>> On Thu, Dec 11, 2025 at 12:40:38PM +0800, Khairul Anuar Romli wrote:
>>>>> Add device tree compatible string support for the Altera Agilex5 AXI DMA
>>>>> controller.
>>>>>
>>>>> Introduces logic to parse the "dma-ranges" property and calculate the
>>>>> actual number of addressable bits (bus width) for the DMA engine. This
>>>>> calculated value is then used to set the coherent mask via
>>>>> 'dma_set_mask_and_coherent()', allowing the driver to correctly handle
>>>>> devices with bus widths less than 64 bits. The addressable bits default to
>>>>> 64 if 'dma-ranges' is not specified or cannot be parsed.
>>>>>
>>>>> Introduce 'addressable_bits' to 'struct axi_dma_chip' to store this value.
>>>>>
>>>>> Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@...era.com>
>>>>> ---
>>>>> Changes in v3:
>>>>> - Refactor the code to align with dma controller device node move
>>>>> to 1 level down.
>>>>> Changes in v2:
>>>>> - Add driver implementation to set the DMA BIT MAST to 40 based on
>>>>> dma-ranges defined in DT.
>>>>> - Add glue for driver and DT.
>>>>> ---
>>>>> .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 69 ++++++++++++++++++-
>>>>> drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 +
>>>>> 2 files changed, 69 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>>>>> index b23536645ff7..96b0a0842ff5 100644
>>>>> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>>>>> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>>>>> @@ -271,7 +271,9 @@ static void axi_dma_hw_init(struct axi_dma_chip *chip)
>>>>> axi_chan_irq_disable(&chip->dw->chan[i], DWAXIDMAC_IRQ_ALL);
>>>>> axi_chan_disable(&chip->dw->chan[i]);
>>>>> }
>>>>> - ret = dma_set_mask_and_coherent(chip->dev, DMA_BIT_MASK(64));
>>>>> +
>>>>> + dev_dbg(chip->dev, "Adressable bus width: %u\n", chip->addressable_bits);
>>>>> + ret = dma_set_mask_and_coherent(chip->dev, DMA_BIT_MASK(chip->addressable_bits));
>>>>> if (ret)
>>>>> dev_warn(chip->dev, "Unable to set coherent mask\n");
>>>>> }
>>>>> @@ -1461,13 +1463,24 @@ static int axi_req_irqs(struct platform_device *pdev, struct axi_dma_chip *chip)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +/* Forward declaration (no size required) */
>>>>> +static const struct of_device_id dw_dma_of_id_table[];
>>>>> +
>>>>> static int dw_probe(struct platform_device *pdev)
>>>>> {
>>>>> struct axi_dma_chip *chip;
>>>>> struct dw_axi_dma *dw;
>>>>> struct dw_axi_dma_hcfg *hdata;
>>>>> struct reset_control *resets;
>>>>> + struct device_node *parent;
>>>>> + const struct of_device_id *match;
>>>>> unsigned int flags;
>>>>> + unsigned int addressable_bits = 64;
>>>>> + unsigned int len_bytes;
>>>>> + unsigned int num_cells;
>>>>> + const __be32 *prop;
>>>>> + u64 bus_width;
>>>>> + u32 *cells;
>>>>> u32 i;
>>>>> int ret;
>>>>>
>>>>> @@ -1483,9 +1496,61 @@ static int dw_probe(struct platform_device *pdev)
>>>>> if (!hdata)
>>>>> return -ENOMEM;
>>>>>
>>>>> + match = of_match_node(dw_dma_of_id_table, pdev->dev.of_node);
>>>>> + if (!match) {
>>>>> + dev_err(&pdev->dev, "Unsupported AXI DMA device\n");
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + parent = of_get_parent(pdev->dev.of_node);
>>>>> + if (parent) {
>>>>> + prop = of_get_property(parent, "dma-ranges", &len_bytes);
>>>>> + if (prop) {
>>>>> + num_cells = len_bytes / sizeof(__be32);
>>>>> + cells = kcalloc(num_cells, sizeof(*cells), GFP_KERNEL);
>>>>> + if (!cells)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + ret = of_property_read_u32(parent, "#address-cells", &i);
>>>>> + if (ret) {
>>>>> + dev_err(&pdev->dev, "missing #address-cells property\n");
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + ret = of_property_read_u32(parent, "#size-cells", &i);
>>>>> + if (ret) {
>>>>> + dev_err(&pdev->dev, "missing #size-cells property\n");
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + if (!of_property_read_u32_array(parent, "dma-ranges",
>>>>> + cells, num_cells)) {
>>>>
>>>> We have common code to parse dma-ranges. Use it and don't implement your
>>>> own.
>>>
>>> Actually, the driver and DT core should take care of all this for you
>>> and there's nothing to do in the driver. A driver only needs to set
>>> the mask for the IP itself and only when >32 bits. The core takes care
>>> of any additional restrictions in the bus.
>>>
>>> Rob
>>
>> The current implementation explicitly set the mask to 64.
>>
>> - ret = dma_set_mask_and_coherent(chip->dev, DMA_BIT_MASK(64));
>>
>> Will the core re-set the mask based on the dma-ranges on DT?
>
> I don't think it changes the mask, but there's also bus ranges which
> get factored in. See struct device.bus_dma_limit and dma_range_map.
>
> Rob
Yes, you're right. the core didn't change the mask that are set from the
driver. I will send the next revision to use common code to get the
dma-ranges and set dma_set_mask_and_coherent accordingly.
Thanks.
Best Regards,
Khairul
Powered by blists - more mailing lists