netdev - Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

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: <2bbd5346-5fe1-16d7-327e-5d94950496f2@denx.de>
Date:   Tue, 25 Jun 2019 00:03:39 +0200
From:   Marek Vasut <marex@...x.de>
To:     netdev@...r.kernel.org
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Tristram Ha <Tristram.Ha@...rochip.com>,
        Woojung Huh <Woojung.Huh@...rochip.com>
Subject: Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

On 6/24/19 12:35 AM, Marek Vasut wrote:
> Add basic SPI regmap support into the driver.
> 
> Previous patches unconver that ksz_spi_write() is always ever called
> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
> check and we can also drop the allocation of the txbuf which is part
> of the driver data and wastes 256 bytes for no reason. Regmap covers
> the whole thing now.
> 
> Signed-off-by: Marek Vasut <marex@...x.de>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Florian Fainelli <f.fainelli@...il.com>
> Cc: Tristram Ha <Tristram.Ha@...rochip.com>
> Cc: Woojung Huh <Woojung.Huh@...rochip.com>

[...]

> +#define KS_SPIOP_FLAG_MASK(opcode)		\
> +	cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))

So the robot is complaining about this. I believe this is correct, as
the mask should be in native endianness on the register and NOT the
native endianness of the CPU.

I think a cast would help here, e.g.:
-	cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
-	(__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
SPI_TURNAROUND_SHIFT))

Does this make sense ?

> +#define KSZ_REGMAP_COMMON(width)					\
> +	{								\
> +		.val_bits = (width),					\
> +		.reg_stride = (width) / 8,				\
> +		.reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,		\
> +		.pad_bits = SPI_TURNAROUND_SHIFT,			\
> +		.max_register = BIT(SPI_ADDR_SHIFT) - 1,		\
> +		.cache_type = REGCACHE_NONE,				\
> +		.read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),	\
> +		.write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),	\

[...]

-- 
Best regards,
Marek Vasut