Re: [GIT PULL v2 2/7] firmware: tegra: Changes for v6.2-rc1

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

On Wed, Nov 23, 2022 at 02:23:36PM +0100, Arnd Bergmann wrote:
> On Wed, Nov 23, 2022, at 12:33, Thierry Reding wrote:
> > On Tue, Nov 22, 2022 at 10:25:50PM +0100, Arnd Bergmann wrote:
> >> On Mon, Nov 21, 2022, at 18:12, Thierry Reding wrote:
> >> >       firmware: tegra: bpmp: Do not support big-endian
> >> 
> >> I pulled the branch, but I think this patch is inconsistent with
> >> our normal approach: Since all ARMv7 and ARMv8 processors can
> >> run with both big-endian and little-endian kernels, we normally
> >> try to keep drivers portable between both ways, even though we
> >> don't expect anyone to actually want a big-endian kernel any
> >> more. Changing portable code to nonportable code doesn't seem
> >> helpful here.
> >
> > The only reason I dropped this is because the driver is in itself
> > inconsistent. Parts of it use byte-swapping for 32-bit values and other
> > parts don't. I was originally going to fix big-endian support but it
> > would've required changes to the BPMP ABI header to avoid sparse
> > warnings in lots of places, then these ABI changes would've needed to
> > trickle up to the canonical source, etc. All of that didn't seem worth
> > the effort if we couldn't even test this in any way. So the easiest fix
> > was to stop pretending and drop the partial support.
> 
> Right
> 
> >> On the other hand, there are already examples of important
> >> drivers that are fundamentally incompatible with big-endian
> >> mode, notably drivers/efi/, which is required on a lot of
> >> machines.
> >> 
> >> You don't have to revert this patch, but it would be helpful
> >> to mark code that is explicitly unportable with a 'depends
> >> on !CPU_BIG_ENDIAN' line in Kconfig. If you agree, I can
> >> add that.
> >
> > Yes, feel free to add that.
> 
> Added this commit to the soc/drivers branch now:
> 
> commit 4ddb1bf1a83783cebdb174b0efaf62f63ad64e0b
> Author: Arnd Bergmann <arnd@arndb.de>
> Date:   Wed Nov 23 14:21:16 2022 +0100
> 
>     tegra: mark BPMP driver as little-endian only
>     
>     The BPMP firmware driver never worked on big-endian kernels, and
>     cannot easily be made portable. Add a dependency to make this clear
>     in case anyone ever wants to try a big-endian kernel on this hardware.
>     
>     Link: https://lore.kernel.org/linux-arm-kernel/Y34FCQ3xTmcjqKRT@orome/
>     Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/firmware/tegra/Kconfig b/drivers/firmware/tegra/Kconfig
> index 1c8ba1f47c7c..cde1ab8bd9d1 100644
> --- a/drivers/firmware/tegra/Kconfig
> +++ b/drivers/firmware/tegra/Kconfig
> @@ -14,6 +14,7 @@ config TEGRA_IVC
>  config TEGRA_BPMP
>  	bool "Tegra BPMP driver"
>  	depends on ARCH_TEGRA && TEGRA_HSP_MBOX && TEGRA_IVC
> +	depends on !CPU_BIG_ENDIAN
>  	help
>  	  BPMP (Boot and Power Management Processor) is designed to off-loading
>  	  the PM functions which include clock/DVFS/thermal/power from the CPU.

Sounds good, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]