| Message ID | 1479257416-29389-12-git-send-email-andre.przywara@arm.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
Hi, On 16.11.2016 01:50, Andre Przywara wrote: > It seems pretty odd that the minimum supported SDHCI frequency is > the maximum frequency shifted _left_ by 9 bits. > Shifting it right by that amount seems to make much more sense. > > Pointed out by GCC 6.2 as the value needs more than 32 bits. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > include/configs/xilinx_zynqmp_ep.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/configs/xilinx_zynqmp_ep.h b/include/configs/xilinx_zynqmp_ep.h > index 8e4b960..d0ce768 100644 > --- a/include/configs/xilinx_zynqmp_ep.h > +++ b/include/configs/xilinx_zynqmp_ep.h > @@ -14,7 +14,7 @@ > #define __CONFIG_ZYNQMP_EP_H > > #define CONFIG_ZYNQ_SDHCI_MAX_FREQ 52000000 > -#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ << 9) > +#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ >> 9) > #define CONFIG_ZYNQ_EEPROM > #define CONFIG_SATA_CEVA > #define CONFIG_ZYNQMP_XHCI_LIST {ZYNQMP_USB0_XHCI_BASEADDR, \ > thanks for the patch. We have fixed that in our internal repo but didn't send this out yet. Here is the link https://github.com/Xilinx/u-boot-xlnx/commit/299ceaf77ee6d5a555ecb5f129bd9248aa981837 Definitely it is good patch and will apply. Thanks, Michal
On 16.11.2016 07:34, Michal Simek wrote: > Hi, > > On 16.11.2016 01:50, Andre Przywara wrote: >> It seems pretty odd that the minimum supported SDHCI frequency is >> the maximum frequency shifted _left_ by 9 bits. >> Shifting it right by that amount seems to make much more sense. >> >> Pointed out by GCC 6.2 as the value needs more than 32 bits. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> include/configs/xilinx_zynqmp_ep.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/configs/xilinx_zynqmp_ep.h b/include/configs/xilinx_zynqmp_ep.h >> index 8e4b960..d0ce768 100644 >> --- a/include/configs/xilinx_zynqmp_ep.h >> +++ b/include/configs/xilinx_zynqmp_ep.h >> @@ -14,7 +14,7 @@ >> #define __CONFIG_ZYNQMP_EP_H >> >> #define CONFIG_ZYNQ_SDHCI_MAX_FREQ 52000000 >> -#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ << 9) >> +#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ >> 9) >> #define CONFIG_ZYNQ_EEPROM >> #define CONFIG_SATA_CEVA >> #define CONFIG_ZYNQMP_XHCI_LIST {ZYNQMP_USB0_XHCI_BASEADDR, \ >> > > thanks for the patch. We have fixed that in our internal repo but didn't > send this out yet. > > Here is the link > https://github.com/Xilinx/u-boot-xlnx/commit/299ceaf77ee6d5a555ecb5f129bd9248aa981837 > > Definitely it is good patch and will apply. ok - it is already the part of my pull request I sent yesterday http://lists.denx.de/pipermail/u-boot/2016-November/272771.html Thanks, Michal
On 16/11/16 06:37, Michal Simek wrote: Hi Michal, > On 16.11.2016 07:34, Michal Simek wrote: >> Hi, >> >> On 16.11.2016 01:50, Andre Przywara wrote: >>> It seems pretty odd that the minimum supported SDHCI frequency is >>> the maximum frequency shifted _left_ by 9 bits. >>> Shifting it right by that amount seems to make much more sense. >>> >>> Pointed out by GCC 6.2 as the value needs more than 32 bits. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> include/configs/xilinx_zynqmp_ep.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/configs/xilinx_zynqmp_ep.h b/include/configs/xilinx_zynqmp_ep.h >>> index 8e4b960..d0ce768 100644 >>> --- a/include/configs/xilinx_zynqmp_ep.h >>> +++ b/include/configs/xilinx_zynqmp_ep.h >>> @@ -14,7 +14,7 @@ >>> #define __CONFIG_ZYNQMP_EP_H >>> >>> #define CONFIG_ZYNQ_SDHCI_MAX_FREQ 52000000 >>> -#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ << 9) >>> +#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ >> 9) >>> #define CONFIG_ZYNQ_EEPROM >>> #define CONFIG_SATA_CEVA >>> #define CONFIG_ZYNQMP_XHCI_LIST {ZYNQMP_USB0_XHCI_BASEADDR, \ >>> >> >> thanks for the patch. We have fixed that in our internal repo but didn't >> send this out yet. >> >> Here is the link >> https://github.com/Xilinx/u-boot-xlnx/commit/299ceaf77ee6d5a555ecb5f129bd9248aa981837 >> >> Definitely it is good patch and will apply. > > ok - it is already the part of my pull request I sent yesterday > http://lists.denx.de/pipermail/u-boot/2016-November/272771.html Even better! Out of curiosity: How did you spot this? Was is actually causing problems or was it GCC 6.x as well? Anyway glad to see it fixed already. Cheers, Andre.
On 16.11.2016 09:14, André Przywara wrote: > On 16/11/16 06:37, Michal Simek wrote: > > Hi Michal, > >> On 16.11.2016 07:34, Michal Simek wrote: >>> Hi, >>> >>> On 16.11.2016 01:50, Andre Przywara wrote: >>>> It seems pretty odd that the minimum supported SDHCI frequency is >>>> the maximum frequency shifted _left_ by 9 bits. >>>> Shifting it right by that amount seems to make much more sense. >>>> >>>> Pointed out by GCC 6.2 as the value needs more than 32 bits. >>>> >>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>>> --- >>>> include/configs/xilinx_zynqmp_ep.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/include/configs/xilinx_zynqmp_ep.h b/include/configs/xilinx_zynqmp_ep.h >>>> index 8e4b960..d0ce768 100644 >>>> --- a/include/configs/xilinx_zynqmp_ep.h >>>> +++ b/include/configs/xilinx_zynqmp_ep.h >>>> @@ -14,7 +14,7 @@ >>>> #define __CONFIG_ZYNQMP_EP_H >>>> >>>> #define CONFIG_ZYNQ_SDHCI_MAX_FREQ 52000000 >>>> -#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ << 9) >>>> +#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ >> 9) >>>> #define CONFIG_ZYNQ_EEPROM >>>> #define CONFIG_SATA_CEVA >>>> #define CONFIG_ZYNQMP_XHCI_LIST {ZYNQMP_USB0_XHCI_BASEADDR, \ >>>> >>> >>> thanks for the patch. We have fixed that in our internal repo but didn't >>> send this out yet. >>> >>> Here is the link >>> https://github.com/Xilinx/u-boot-xlnx/commit/299ceaf77ee6d5a555ecb5f129bd9248aa981837 >>> >>> Definitely it is good patch and will apply. >> >> ok - it is already the part of my pull request I sent yesterday >> http://lists.denx.de/pipermail/u-boot/2016-November/272771.html > > Even better! > Out of curiosity: How did you spot this? Was is actually causing > problems or was it GCC 6.x as well? > > Anyway glad to see it fixed already. Yes, it was causing the problem on emulation platform which hasn't been detected by testing. Thanks, Michal
diff --git a/include/configs/xilinx_zynqmp_ep.h b/include/configs/xilinx_zynqmp_ep.h index 8e4b960..d0ce768 100644 --- a/include/configs/xilinx_zynqmp_ep.h +++ b/include/configs/xilinx_zynqmp_ep.h @@ -14,7 +14,7 @@ #define __CONFIG_ZYNQMP_EP_H #define CONFIG_ZYNQ_SDHCI_MAX_FREQ 52000000 -#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ << 9) +#define CONFIG_ZYNQ_SDHCI_MIN_FREQ (CONFIG_ZYNQ_SDHCI_MAX_FREQ >> 9) #define CONFIG_ZYNQ_EEPROM #define CONFIG_SATA_CEVA #define CONFIG_ZYNQMP_XHCI_LIST {ZYNQMP_USB0_XHCI_BASEADDR, \
It seems pretty odd that the minimum supported SDHCI frequency is the maximum frequency shifted _left_ by 9 bits. Shifting it right by that amount seems to make much more sense. Pointed out by GCC 6.2 as the value needs more than 32 bits. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- include/configs/xilinx_zynqmp_ep.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)