Re: [PATCH] spi: prevent overriding established bus settings
niedz., 26 sty 2020 o 14:30 Jagan Teki napisał(a): > > On Thu, Nov 21, 2019 at 10:09 AM Marcin Wojtas wrote: > > > > The SPI stack relies on a proper bus speed/mode configuration > > by calling dm_spi_claim_bus(). However the hitherto code > > allowed to accidentally override those settings in > > the spi_get_bus_and_cs() routine. > > > > The initially established speed could be discarded by using > > the slave platdata, which turned out to be an issue on > > the platforms whose slave maximum supported frequency > > is not on par with the maximum frequency of the bus controller. > > > > This patch fixes above issue by configuring the bus from > > spi_get_bus_and_cs() only in case it was not done before. > > > > Signed-off-by: Marcin Wojtas > > --- > > Applied to u-boot-spi/master Great, thanks! Marcin
Re: [U-Boot] [PATCH] spi: prevent overriding established bus settings
Hi Jagan, sob., 28 gru 2019 o 03:27 Simon Glass napisał(a): > > On Thu, 21 Nov 2019 at 04:45, Marcin Wojtas wrote: > > > > The SPI stack relies on a proper bus speed/mode configuration > > by calling dm_spi_claim_bus(). However the hitherto code > > allowed to accidentally override those settings in > > the spi_get_bus_and_cs() routine. > > > > The initially established speed could be discarded by using > > the slave platdata, which turned out to be an issue on > > the platforms whose slave maximum supported frequency > > is not on par with the maximum frequency of the bus controller. > > > > This patch fixes above issue by configuring the bus from > > spi_get_bus_and_cs() only in case it was not done before. > > > > Signed-off-by: Marcin Wojtas > > --- > > drivers/spi/spi-uclass.c | 20 +++- > > 1 file changed, 11 insertions(+), 9 deletions(-) > > Reviewed-by: Simon Glass Is there a chance to merge the patch, or do you have any objections? Best regards, Marcin
Re: [PATCH] spi: prevent overriding established bus settings
Hi,
Any comments on the patch?
Best regards,
Marcin
czw., 21 lis 2019 o 10:29 Marcin Wojtas napisał(a):
> + Simon
>
> czw., 21 lis 2019 o 05:39 Marcin Wojtas napisał(a):
>
>> The SPI stack relies on a proper bus speed/mode configuration
>> by calling dm_spi_claim_bus(). However the hitherto code
>> allowed to accidentally override those settings in
>> the spi_get_bus_and_cs() routine.
>>
>> The initially established speed could be discarded by using
>> the slave platdata, which turned out to be an issue on
>> the platforms whose slave maximum supported frequency
>> is not on par with the maximum frequency of the bus controller.
>>
>> This patch fixes above issue by configuring the bus from
>> spi_get_bus_and_cs() only in case it was not done before.
>>
>> Signed-off-by: Marcin Wojtas
>> ---
>> drivers/spi/spi-uclass.c | 20 +++-
>> 1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
>> index 76c4b53..fcbe532 100644
>> --- a/drivers/spi/spi-uclass.c
>> +++ b/drivers/spi/spi-uclass.c
>> @@ -296,6 +296,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed,
>> int mode,
>> {
>> struct udevice *bus, *dev;
>> struct dm_spi_slave_platdata *plat;
>> + struct spi_slave *slave;
>> bool created = false;
>> int ret;
>>
>> @@ -351,19 +352,20 @@ int spi_get_bus_and_cs(int busnum, int cs, int
>> speed, int mode,
>> slave->dev = dev;
>> }
>>
>> - plat = dev_get_parent_platdata(dev);
>> + slave = dev_get_parent_priv(dev);
>>
>> - /* get speed and mode from platdata when available */
>> - if (plat->max_hz) {
>> - speed = plat->max_hz;
>> - mode = plat->mode;
>> + /*
>> +* In case the operation speed is not yet established by
>> +* dm_spi_claim_bus() ensure the bus is configured properly.
>> +*/
>> + if (!slave->speed) {
>> + ret = spi_claim_bus(slave);
>> + if (ret)
>> + goto err;
>> }
>> - ret = spi_set_speed_mode(bus, speed, mode);
>> - if (ret)
>> - goto err;
>>
>> *busp = bus;
>> - *devp = dev_get_parent_priv(dev);
>> + *devp = slave;
>> debug("%s: bus=%p, slave=%p\n", __func__, bus, *devp);
>>
>> return 0;
>> --
>> 2.7.4
>>
>>
Re: [U-Boot] [PATCH] spi: prevent overriding established bus settings
+ Simon
czw., 21 lis 2019 o 05:39 Marcin Wojtas napisał(a):
> The SPI stack relies on a proper bus speed/mode configuration
> by calling dm_spi_claim_bus(). However the hitherto code
> allowed to accidentally override those settings in
> the spi_get_bus_and_cs() routine.
>
> The initially established speed could be discarded by using
> the slave platdata, which turned out to be an issue on
> the platforms whose slave maximum supported frequency
> is not on par with the maximum frequency of the bus controller.
>
> This patch fixes above issue by configuring the bus from
> spi_get_bus_and_cs() only in case it was not done before.
>
> Signed-off-by: Marcin Wojtas
> ---
> drivers/spi/spi-uclass.c | 20 +++-
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> index 76c4b53..fcbe532 100644
> --- a/drivers/spi/spi-uclass.c
> +++ b/drivers/spi/spi-uclass.c
> @@ -296,6 +296,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed,
> int mode,
> {
> struct udevice *bus, *dev;
> struct dm_spi_slave_platdata *plat;
> + struct spi_slave *slave;
> bool created = false;
> int ret;
>
> @@ -351,19 +352,20 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int mode,
> slave->dev = dev;
> }
>
> - plat = dev_get_parent_platdata(dev);
> + slave = dev_get_parent_priv(dev);
>
> - /* get speed and mode from platdata when available */
> - if (plat->max_hz) {
> - speed = plat->max_hz;
> - mode = plat->mode;
> + /*
> +* In case the operation speed is not yet established by
> +* dm_spi_claim_bus() ensure the bus is configured properly.
> +*/
> + if (!slave->speed) {
> + ret = spi_claim_bus(slave);
> + if (ret)
> + goto err;
> }
> - ret = spi_set_speed_mode(bus, speed, mode);
> - if (ret)
> - goto err;
>
> *busp = bus;
> - *devp = dev_get_parent_priv(dev);
> + *devp = slave;
> debug("%s: bus=%p, slave=%p\n", __func__, bus, *devp);
>
> return 0;
> --
> 2.7.4
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] spi: prevent overriding established bus settings
The SPI stack relies on a proper bus speed/mode configuration
by calling dm_spi_claim_bus(). However the hitherto code
allowed to accidentally override those settings in
the spi_get_bus_and_cs() routine.
The initially established speed could be discarded by using
the slave platdata, which turned out to be an issue on
the platforms whose slave maximum supported frequency
is not on par with the maximum frequency of the bus controller.
This patch fixes above issue by configuring the bus from
spi_get_bus_and_cs() only in case it was not done before.
Signed-off-by: Marcin Wojtas
---
drivers/spi/spi-uclass.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index 76c4b53..fcbe532 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -296,6 +296,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int
mode,
{
struct udevice *bus, *dev;
struct dm_spi_slave_platdata *plat;
+ struct spi_slave *slave;
bool created = false;
int ret;
@@ -351,19 +352,20 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int
mode,
slave->dev = dev;
}
- plat = dev_get_parent_platdata(dev);
+ slave = dev_get_parent_priv(dev);
- /* get speed and mode from platdata when available */
- if (plat->max_hz) {
- speed = plat->max_hz;
- mode = plat->mode;
+ /*
+* In case the operation speed is not yet established by
+* dm_spi_claim_bus() ensure the bus is configured properly.
+*/
+ if (!slave->speed) {
+ ret = spi_claim_bus(slave);
+ if (ret)
+ goto err;
}
- ret = spi_set_speed_mode(bus, speed, mode);
- if (ret)
- goto err;
*busp = bus;
- *devp = dev_get_parent_priv(dev);
+ *devp = slave;
debug("%s: bus=%p, slave=%p\n", __func__, bus, *devp);
return 0;
--
2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC] Reserve ATF memory on Marvell Armdada 3700/7K/8K
Hi Patrick, You can refer to the mainline edk2 patch: https://github.com/tianocore/edk2-platforms/commit/bf1c4a2cf8024669d1748e78c7e417433f85707e Best regards, Marcin wt., 12 lut 2019 o 10:38 Alexander Graf napisał(a): > On 02/04/2019 05:51 PM, Patrick Wildt wrote: > > On Fri, Apr 06, 2018 at 12:22:03PM +0200, Alexander Graf wrote: > >> On 31.03.18 16:13, Mark Kettenis wrote: > >>> Currently U-Boot doesn't make any effort to reserve the memory used by > >>> ARM Trusted Firmware on these platforms. The result is that the > >>> memory is listed as available in the EFI memory map. And as soon as a > >>> loaded kernel tries to use this memory things explode. I've seen this > >>> with the OpenBSD kernel. But I totally expect a Linux kernel to > >>> suffer the same fate. > >>> > >>> I'm currently using the diff below, but it is not entirely clear to me > >>> if arch_early_init_r() is the appropriate place to do this. I'm also > >>> wondering whether the block should also be marked as reserved in the > >>> FDT using fdt_add_mem_rsv(). If the latter is required this probably > >>> needs to be done by ft_board_setup() or ft_system_setup(). > >>> > >>> The address and size of the region have been taken from Marvell's ATF > >>> fork at > >>> > >>>https://github.com/MarvellEmbeddedProcessors/atf-marvell > >>> > >>> The memory layout is defined in > >>> > >>>plat/marvell/a8k/common/include/platform_def.h > >>> > >>> where there are lots of defines and a diagram that attempt to describe > >>> the memory. It is not entirely obvious to me what part needs to be > >>> reserved. But 0x040-0x0420 works. > >>> > >>> > >>> > >>> > >>> diff --git a/arch/arm/mach-mvebu/arm64-common.c > b/arch/arm/mach-mvebu/arm64-common.c > >>> index 3c84043a2c..895cd2852f 100644 > >>> --- a/arch/arm/mach-mvebu/arm64-common.c > >>> +++ b/arch/arm/mach-mvebu/arm64-common.c > >>> @@ -95,5 +95,11 @@ int arch_early_init_r(void) > >>> pci_init(); > >>> #endif > >>> > >>> +#ifdef CONFIG_EFI_LOADER > >>> + /* Reserve trusted SRAM section */ > >>> + efi_add_memory_map(0x0400, 0x0020 >> EFI_PAGE_SHIFT, > >>> + EFI_RESERVED_MEMORY_TYPE, false); > >> I also forgot to comment here. 2MB is probably not enough: > >> > >> > >> > https://github.com/MarvellEmbeddedProcessors/atf-marvell/blob/atf-v1.3-armada-17.10/plat/marvell/a8k/common/include/platform_def.h#L110 > >> > >> That sounds more like it should span 65MB (PLAT_MARVELL_TRUSTED_ROM_SIZE > >> plus 0x10). > >> > >> Looking at what edk2 does, it seems to use the same range as you: > >> > >> > >> > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/blob/marvell-armada-wip/Platforms/Marvell/Armada/Armada.dsc.inc#L347 > >> > >> So let's also CC Marcin :). Maybe his range is too short! > >> > >> > >> Alex > >> ___ > >> U-Boot mailing list > >> U-Boot@lists.denx.de > >> https://lists.denx.de/listinfo/u-boot > > The current ATF and EDK2 branch from Marvell seem to reserve a bit more. > > I just had the same issue and am using this diff. This then creates the > > same map of usable memory as EDK2 does. > > Could you please resend as proper patch (with SoB, separate email) with > a reference to the edk2 code in the commit message? > > > Thanks, > > Alex > > > > > Best regards, > > Patrick > > > > diff --git a/arch/arm/mach-mvebu/arm64-common.c > b/arch/arm/mach-mvebu/arm64-common.c > > index 47bbf69944..56c0d3f6b9 100644 > > --- a/arch/arm/mach-mvebu/arm64-common.c > > +++ b/arch/arm/mach-mvebu/arm64-common.c > > @@ -14,6 +14,7 @@ > > #include > > #include > > #include > > +#include > > > > DECLARE_GLOBAL_DATA_PTR; > > > > @@ -142,5 +143,11 @@ int arch_early_init_r(void) > > pci_init(); > > #endif > > > > +#ifdef CONFIG_EFI_LOADER > > + /* Reserve trusted SRAM section */ > > + efi_add_memory_map(0x0400, 0x0140 >> EFI_PAGE_SHIFT, > > +EFI_RESERVED_MEMORY_TYPE, false); > > +#endif > > + > > return 0; > > } > > > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
