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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a41c9684-6562-45bb-b009-1fe7b265f60e@linaro.org>
Date: Tue, 21 May 2024 12:32:10 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Witold Sadowski <wsadowski@...vell.com>, linux-kernel@...r.kernel.org,
 linux-spi@...r.kernel.org, devicetree@...r.kernel.org
Cc: broonie@...nel.org, robh@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 conor+dt@...nel.org, pthombar@...ence.com
Subject: Re: [PATCH v4 3/5] spi: cadence: Add Marvell xSPI IP overlay changes

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@...vell.com>
> ---


..

>  
>  	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