Re: [PATCH v4 3/5] spi: cadence: Add Marvell xSPI IP overlay changes

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 09/05/2024 03:05, Witold Sadowski wrote:
> Add support for basic v2 Marvell overlay block. Support for basic
> operation is added here: clock configuration, PHY configuration,
> interrupt configuration(enabling)
> Clock divider block is build on top of Cadence xSPI IP, and divides
> external 800MHz clock. It allows only for a few different clock speeds
> starting from 6.25MHz up to 200MHz.
> PHY configuration can be read from device-tree, if parameter is not
> present - safe defaults will be used..
> In addition to handle interrupt propoerly driver must clear MSI-X
> interrupt bit, in addition to clearing xSPI interrupt bit. Interrupt
> masking must be disabled.

Please use full sentences, properly wrapped, continued, readable. There
are typos above, double full-stops and it looks like one sentence per
paragraph... or some sort of list.

> 
> Signed-off-by: Witold Sadowski <wsadowski@xxxxxxxxxxx>
> ---


...

>  
>  	cdns_xspi = spi_controller_get_devdata(host);
> @@ -565,23 +809,27 @@ static int cdns_xspi_probe(struct platform_device *pdev)
>  	init_completion(&cdns_xspi->auto_cmd_complete);
>  	init_completion(&cdns_xspi->sdma_complete);
>  
> +	cdns_xspi->mrvl_hw_overlay = drv_data->mrvl_hw_overlay;
> +
>  	ret = cdns_xspi_of_get_plat_data(pdev);
>  	if (ret)
>  		return -ENODEV;
>  
> -	cdns_xspi->iobase = devm_platform_ioremap_resource_byname(pdev, "io");
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	cdns_xspi->iobase = devm_ioremap_resource(dev, res);

Why are you changing this to two calls? The wrapper is there on purpose.

Anyway, does not look related to this patch.


>  	if (IS_ERR(cdns_xspi->iobase)) {
>  		dev_err(dev, "Failed to remap controller base address\n");
>  		return PTR_ERR(cdns_xspi->iobase);
>  	}
>  
> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sdma");
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	cdns_xspi->sdmabase = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(cdns_xspi->sdmabase))
>  		return PTR_ERR(cdns_xspi->sdmabase);
>  	cdns_xspi->sdmasize = resource_size(res);
>  
> -	cdns_xspi->auxbase = devm_platform_ioremap_resource_byname(pdev, "aux");
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	cdns_xspi->auxbase = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(cdns_xspi->auxbase)) {
>  		dev_err(dev, "Failed to remap AUX address\n");
>  		return PTR_ERR(cdns_xspi->auxbase);
> @@ -598,8 +846,12 @@ static int cdns_xspi_probe(struct platform_device *pdev)
>  		return ret;
>  	}


Best regards,
Krzysztof