mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4] Kindle i.MX50 improvements
@ 2018-09-02 21:21 Roland Hieber
  2018-09-02 21:21 ` [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning Roland Hieber
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Roland Hieber @ 2018-09-02 21:21 UTC (permalink / raw)
  To: barebox; +Cc: Roland Hieber

From: Roland Hieber <rohieb@rohieb.name>

For a project, dug up my old Kindle 4 Wi-Fi, and tried to get a barebox
running on it. Here is the output – nothing big, but still useful.

Patch 1/4 is only loosely related, but was written in the process, and
the time that it took me to write these four lines mentioning that fact
could probably have been spent better by sending it separately. But
well, now I've already written these four lines instead.

Roland Hieber (4):
  pinctrl: imx-iomux-v3: fix compiler warning
  ARM: i.MX: Kindle 4/5 is based on Device Tree, select it in Kconfig
  ARM: i.MX: add defconfig for the Kindle i.MX50 boards
  Documentation: i.MX: update Kindle 4/5 board documentation

 .../boards/imx/amazon-kindle-4-5.rst          | 81 ++++++++++---------
 arch/arm/configs/kindle-mx50_defconfig        | 70 ++++++++++++++++
 arch/arm/mach-imx/Kconfig                     |  1 +
 drivers/pinctrl/imx-iomux-v3.c                |  2 +-
 4 files changed, 113 insertions(+), 41 deletions(-)
 create mode 100644 arch/arm/configs/kindle-mx50_defconfig

-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning
  2018-09-02 21:21 [PATCH 0/4] Kindle i.MX50 improvements Roland Hieber
@ 2018-09-02 21:21 ` Roland Hieber
  2018-09-03  4:46   ` Sam Ravnborg
  2018-09-02 21:21 ` [PATCH 2/4] ARM: i.MX: Kindle 4/5 is based on Device Tree, select it in Kconfig Roland Hieber
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Roland Hieber @ 2018-09-02 21:21 UTC (permalink / raw)
  To: barebox; +Cc: Roland Hieber

From: Roland Hieber <rohieb@rohieb.name>

Fix a warning while compiling with GCC 5.4.0 (OSELAS.Toolchain 2016.02):

    drivers/pinctrl/imx-iomux-v3.c: In function 'imx_iomux_v3_set_state':
    drivers/pinctrl/imx-iomux-v3.c:153:13: warning: 'share_conf_val' may be used uninitialized in this function [-Wmaybe-uninitialized]
        conf_val &= ~IMX_PAD_SION;
                 ^
The relevant code section at line 153 is:

148:		u32 conf_val = share_conf ?
149:			share_conf_val : be32_to_cpu(*list++);
150:
151:		if (conf_val & IMX_PAD_SION) {
152:			mux_val |= IOMUXC_CONFIG_SION;
153:			conf_val &= ~IMX_PAD_SION;
154:		}

share_conf_val is only initialized in an if branch above that section.

Signed-off-by: Roland Hieber <rohieb@rohieb.name>
---
 drivers/pinctrl/imx-iomux-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/imx-iomux-v3.c b/drivers/pinctrl/imx-iomux-v3.c
index d176199c52..0ab97040e0 100644
--- a/drivers/pinctrl/imx-iomux-v3.c
+++ b/drivers/pinctrl/imx-iomux-v3.c
@@ -87,7 +87,7 @@ static int imx_iomux_v3_set_state(struct pinctrl_device *pdev, struct device_nod
 	const bool share_conf = iomux->flags & SHARE_CONF;
 	int npins, size, i, fsl_pin_size;
 	const char *name;
-	u32 share_conf_val;
+	u32 share_conf_val = 0;
 
 	dev_dbg(iomux->pinctrl.dev, "set state: %s\n", np->full_name);
 
-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] ARM: i.MX: Kindle 4/5 is based on Device Tree, select it in Kconfig
  2018-09-02 21:21 [PATCH 0/4] Kindle i.MX50 improvements Roland Hieber
  2018-09-02 21:21 ` [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning Roland Hieber
@ 2018-09-02 21:21 ` Roland Hieber
  2018-09-02 21:21 ` [PATCH 3/4] ARM: i.MX: add defconfig for the Kindle i.MX50 boards Roland Hieber
  2018-09-02 21:21 ` [PATCH 4/4] Documentation: i.MX: update Kindle 4/5 board documentation Roland Hieber
  3 siblings, 0 replies; 9+ messages in thread
From: Roland Hieber @ 2018-09-02 21:21 UTC (permalink / raw)
  To: barebox; +Cc: Roland Hieber

From: Roland Hieber <rohieb@rohieb.name>

Without OFTREE, no ./scripts/dtc and none of the device trees will be
built, and compilation will fail because the board code references it.

Signed-off-by: Roland Hieber <rohieb@rohieb.name>
---
 arch/arm/mach-imx/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 7cb9138d24..d57ea369a8 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -242,6 +242,7 @@ config MACH_KINDLE_MX50
 	select MFD_MC13XXX
 	select ARM_BOARD_APPEND_ATAG
 	select ARM_LINUX
+	select OFTREE
 	help
 	  Say Y here if you are using the fourth or fifth generation Amazon
 	  Kindle Model No. D01100 (Kindle Wi-Fi), D01200 (Kindle Touch) or
-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] ARM: i.MX: add defconfig for the Kindle i.MX50 boards
  2018-09-02 21:21 [PATCH 0/4] Kindle i.MX50 improvements Roland Hieber
  2018-09-02 21:21 ` [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning Roland Hieber
  2018-09-02 21:21 ` [PATCH 2/4] ARM: i.MX: Kindle 4/5 is based on Device Tree, select it in Kconfig Roland Hieber
@ 2018-09-02 21:21 ` Roland Hieber
  2018-09-02 21:21 ` [PATCH 4/4] Documentation: i.MX: update Kindle 4/5 board documentation Roland Hieber
  3 siblings, 0 replies; 9+ messages in thread
From: Roland Hieber @ 2018-09-02 21:21 UTC (permalink / raw)
  To: barebox; +Cc: Roland Hieber

From: Roland Hieber <rohieb@rohieb.name>

Pin down the minimal config needed for a Kindle 4/5 device to boot into
the vendor Linux.

As the board documentation says, there is an upper limit of ~240 KB for
the bootloader partition on those devices. Using XZ image compression
instead of the default LZO reduced the file size from 214 KB to 172 KB,
so enabling that too.

Signed-off-by: Roland Hieber <rohieb@rohieb.name>
---
 arch/arm/configs/kindle-mx50_defconfig | 70 ++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 arch/arm/configs/kindle-mx50_defconfig

diff --git a/arch/arm/configs/kindle-mx50_defconfig b/arch/arm/configs/kindle-mx50_defconfig
new file mode 100644
index 0000000000..31bfc9c06b
--- /dev/null
+++ b/arch/arm/configs/kindle-mx50_defconfig
@@ -0,0 +1,70 @@
+CONFIG_ARCH_IMX=y
+CONFIG_IMX_MULTI_BOARDS=y
+CONFIG_MACH_KINDLE_MX50=y
+CONFIG_THUMB2_BAREBOX=y
+CONFIG_ARM_OPTIMZED_STRING_FUNCTIONS=y
+CONFIG_ARM_UNWIND=y
+CONFIG_MMU=y
+CONFIG_MMU_EARLY=y
+CONFIG_MALLOC_SIZE=0x0
+CONFIG_MALLOC_TLSF=y
+CONFIG_KALLSYMS=y
+CONFIG_RELOCATABLE=y
+CONFIG_HUSH_FANCY_PROMPT=y
+CONFIG_CMDLINE_EDITING=y
+CONFIG_AUTO_COMPLETE=y
+CONFIG_FLEXIBLE_BOOTARGS=y
+CONFIG_BOOTM_SHOW_TYPE=y
+CONFIG_BOOTM_VERBOSE=y
+CONFIG_BOOTM_INITRD=y
+CONFIG_BOOTM_OFTREE=y
+CONFIG_CONSOLE_ACTIVATE_ALL=y
+CONFIG_CONSOLE_ALLOW_COLOR=y
+CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y
+CONFIG_CMD_DMESG=y
+CONFIG_LONGHELP=y
+CONFIG_CMD_MEMINFO=y
+CONFIG_CMD_BOOT=y
+CONFIG_CMD_RESET=y
+CONFIG_CMD_AUTOMOUNT=y
+CONFIG_CMD_EXPORT=y
+CONFIG_CMD_GLOBAL=y
+CONFIG_CMD_PRINTENV=y
+CONFIG_CMD_MAGICVAR=y
+CONFIG_CMD_MAGICVAR_HELP=y
+CONFIG_CMD_DETECT=y
+CONFIG_CMD_FLASH=y
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_USBGADGET=y
+CONFIG_CMD_OF_DUMP=y
+CONFIG_CMD_OFTREE=y
+CONFIG_OFDEVICE=y
+CONFIG_OF_GPIO=y
+CONFIG_OF_BAREBOX_DRIVERS=y
+CONFIG_MTD=y
+CONFIG_MTD_WRITE=y
+CONFIG_MTD_M25P80=y
+CONFIG_MTD_SPI_NOR=y
+CONFIG_USB=y
+CONFIG_USB_HOST=y
+CONFIG_USB_IMX_CHIPIDEA=y
+CONFIG_USB_EHCI=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_DRIVER_ARC=y
+CONFIG_USB_GADGET_SERIAL=y
+CONFIG_USB_GADGET_FASTBOOT=y
+CONFIG_MCI=y
+CONFIG_MCI_INFO=y
+CONFIG_MCI_WRITE=y
+CONFIG_MCI_MMC_BOOT_PARTITIONS=y
+CONFIG_MCI_IMX_ESDHC=y
+CONFIG_PINCTRL=y
+CONFIG_REGULATOR=y
+CONFIG_REGULATOR_FIXED=y
+CONFIG_GENERIC_PHY=y
+CONFIG_USB_NOP_XCEIV=y
+CONFIG_FS_AUTOMOUNT=y
+CONFIG_IMAGE_COMPRESSION_XZKERN=y
-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] Documentation: i.MX: update Kindle 4/5 board documentation
  2018-09-02 21:21 [PATCH 0/4] Kindle i.MX50 improvements Roland Hieber
                   ` (2 preceding siblings ...)
  2018-09-02 21:21 ` [PATCH 3/4] ARM: i.MX: add defconfig for the Kindle i.MX50 boards Roland Hieber
@ 2018-09-02 21:21 ` Roland Hieber
  3 siblings, 0 replies; 9+ messages in thread
From: Roland Hieber @ 2018-09-02 21:21 UTC (permalink / raw)
  To: barebox; +Cc: Roland Hieber

From: Roland Hieber <rohieb@rohieb.name>

- complete the instructions to get into USB bootloader mode
- put hardware info to the top, and split into sections
- add info about the new defconfig
- remove build size hints, which have been applied to the defconfig
- transform literal blocks into code blocks

Signed-off-by: Roland Hieber <rohieb@rohieb.name>
---
 .../boards/imx/amazon-kindle-4-5.rst          | 81 ++++++++++---------
 1 file changed, 41 insertions(+), 40 deletions(-)

diff --git a/Documentation/boards/imx/amazon-kindle-4-5.rst b/Documentation/boards/imx/amazon-kindle-4-5.rst
index bc6bf2609b..58f38a058e 100644
--- a/Documentation/boards/imx/amazon-kindle-4-5.rst
+++ b/Documentation/boards/imx/amazon-kindle-4-5.rst
@@ -1,7 +1,8 @@
-Amazon Kindle 4/5 Model No. D01100, D01200 and EY21
-===================================================
+Amazon Kindle 4/5 (Wi-Fi/No-Touch, Touch and Paperwhite)
+========================================================
 
-The Kindle Model No. D01100 (Kindle Wi-Fi), D01200 (Kindle Touch)
+The Kindle Models No. D01100 (Kindle Wi-Fi, also known as No-Touch or K4NT),
+D01200 (Kindle Touch)
 and EY21 (Paperwhite) are refered as the Kindle 4th and 5th generation.
 Those e-book readers share a common set of hardware:
 
@@ -15,55 +16,55 @@ while the newer EY21 uses 256MiB of LPDDR2.
 The devices boot up in internal boot mode from an eMMC boot partition and
 are shipped with a vendor modified u-boot imximage based on u-boot v2009.08.
 
-To upload and run a new bootloader the older devices can be put into
-USB-downloader mode by the SOC microcode when a specific key is pressed during
-startup:
+This device is battery-powered and there is no way to switch the device off.
+When the device is inactive, the Kindle software will first reduce the
+power consumption to a few milliamps of battery power, after some minutes
+the power consumption is further reduced to about 550 microamps. Switching
+on iomux pullups may significantly reduce your standby-time.
 
-* the fiveway down button on the model D01100
-* the home button on model D01200
+Building barebox
+----------------
 
-A new USB device "NS Blank CODEX" should appear, barebox may be uploaded using
+``make kindle-mx50_defconfig`` should get you a working config.
 
-::
+Uploading barebox
+-----------------
 
-        $ scripts/imx/imx-usb-loader barebox-kindle-d01100.img
-        $ scripts/imx/imx-usb-loader barebox-kindle-d01200.img
+To upload and run a new bootloader, the older devices can be put into
+USB bootloader mode by the SoC microcode:
 
-Hint: keep the select button pressed down to get the barebox USB console.
+1. Connect the Kindle to your host computer with a USB cable.
+2. Power down the device by holding the power button until the power LED goes
+   dark (about 10 seconds).
+4. Hold the power button, and hold down a device-specific special key:
+   * the fiveway down button on the model D01100
+   * the home button on model D01200
+4. Then release the power button, but still hold the special key.
+5. A new USB device named ``NS Blank CODEX`` should appear on your host computer.
+   You can now release the special button.
+7. Finally, upload barebox to the Kindle by using:
 
-Barebox may be used as drop-in replacement for the shipped bootloader, when
-the imximg fits into 258048 bytes. When installing the barebox imximg on
-the eMMC, take care not to overwrite the vendor supplied serial numbers stored
-on the eMMC,
-e.g. for the D01100 just write the imx-header and the application section::
+   .. code-block:: console
 
-        loady -t usbserial
-        memcpy -b -s barebox-kindle-d01100.img -d /dev/disk0.boot0.imx_header 1024 0 2048
-        memcpy -b -s barebox-kindle-d01100.img -d /dev/disk0.boot0.self 4096 0 253952
+        $ scripts/imx/imx-usb-loader barebox-kindle-d01100.img
+        $ scripts/imx/imx-usb-loader barebox-kindle-d01200.img
 
-Note: a USB serial ACM console will be launched by a barebox init script
-when
+Additionally, a USB serial ACM console will be launched by a barebox init script
+when:
 
 * the cursor select key is pressed during startup of model D01100
 * the home button is pressed within a second after startup of model D01200.
-  If you press the home button during startup, you will enter USB boot mode.
+  (If you press the home button during startup, you will enter USB boot mode.)
 * the EY21 has no keys to press, a USB console will be launched for 10s.
 
-This device is battery-powered and there is no way to switch the device off.
-When the device is inactive, the kindle software will first reduce the
-power consumption to a few milliamps of battery power, after some minutes
-the power consumption is further reduced to about 550 microamps. Switching
-on iomux pullups may significantly reduce your standby-time.
-
-Hints to reduce the build image size
-------------------------------------
+Barebox may be used as drop-in replacement for the shipped bootloader, when
+the imximg fits into 258048 bytes. When installing the barebox imximg on
+the eMMC, take care not to overwrite the vendor supplied serial numbers stored
+on the eMMC,
+e.g. for the D01100 just write the imx-header and the application section:
 
-Note that a drop-in replacement barebox imximage must not exceed 258048 bytes
-since the space behind it is in use. Hence, don't build in drivers and FS
-that are not required, e.g.
-``NET, DISK_AHCI, DISK_INTF_PLATFORM_IDE, DISK_ATA, VIDEO, PWM, LED,
-USB_STORAGE, USB_ULPI, NAND, MTD_UBI, FS_UBIFS, MFD_MC34704, MFD_MC9SDZ60,
-MFD_STMPE, EEPROM_AT25, EEPROM_AT24, KEYBOARD_GPIO, PARTITION_DISK_EFI``
+.. code-block:: console
 
-Also unselect support for other boards to get rid of their dependencies.
-Further select ``IMAGE_COMPRESSION_XZKERN``.
+        $ loady -t usbserial
+        $ memcpy -b -s barebox-kindle-d01100.img -d /dev/disk0.boot0.imx_header 1024 0 2048
+        $ memcpy -b -s barebox-kindle-d01100.img -d /dev/disk0.boot0.self 4096 0 253952
-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning
  2018-09-02 21:21 ` [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning Roland Hieber
@ 2018-09-03  4:46   ` Sam Ravnborg
  2018-09-03 13:50     ` Roland Hieber
  0 siblings, 1 reply; 9+ messages in thread
From: Sam Ravnborg @ 2018-09-03  4:46 UTC (permalink / raw)
  To: Roland Hieber; +Cc: barebox, Roland Hieber

Hi Roland.

On Sun, Sep 02, 2018 at 11:21:20PM +0200, Roland Hieber wrote:
> From: Roland Hieber <rohieb@rohieb.name>
> 
> Fix a warning while compiling with GCC 5.4.0 (OSELAS.Toolchain 2016.02):
> 
>     drivers/pinctrl/imx-iomux-v3.c: In function 'imx_iomux_v3_set_state':
>     drivers/pinctrl/imx-iomux-v3.c:153:13: warning: 'share_conf_val' may be used uninitialized in this function [-Wmaybe-uninitialized]
>         conf_val &= ~IMX_PAD_SION;
>                  ^
> The relevant code section at line 153 is:
> 
> 148:		u32 conf_val = share_conf ?
> 149:			share_conf_val : be32_to_cpu(*list++);
> 150:
> 151:		if (conf_val & IMX_PAD_SION) {
> 152:			mux_val |= IOMUXC_CONFIG_SION;
> 153:			conf_val &= ~IMX_PAD_SION;
> 154:		}

In this code snip we only see that share_conf_val is used (line 149),
it is not assigned.
So we do not really see the context of your message in the code snip.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning
  2018-09-03  4:46   ` Sam Ravnborg
@ 2018-09-03 13:50     ` Roland Hieber
  2018-09-03 20:14       ` Sam Ravnborg
  0 siblings, 1 reply; 9+ messages in thread
From: Roland Hieber @ 2018-09-03 13:50 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: barebox

On Mon, Sep 03, 2018 at 06:46:57AM +0200, Sam Ravnborg wrote:
> Hi Roland.
> 
> On Sun, Sep 02, 2018 at 11:21:20PM +0200, Roland Hieber wrote:
> > From: Roland Hieber <rohieb@rohieb.name>
> > 
> > Fix a warning while compiling with GCC 5.4.0 (OSELAS.Toolchain 2016.02):
> > 
> >     drivers/pinctrl/imx-iomux-v3.c: In function 'imx_iomux_v3_set_state':
> >     drivers/pinctrl/imx-iomux-v3.c:153:13: warning: 'share_conf_val' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >         conf_val &= ~IMX_PAD_SION;
> >                  ^
> > The relevant code section at line 153 is:
> > 
> > 148:		u32 conf_val = share_conf ?
> > 149:			share_conf_val : be32_to_cpu(*list++);
> > 150:
> > 151:		if (conf_val & IMX_PAD_SION) {
> > 152:			mux_val |= IOMUXC_CONFIG_SION;
> > 153:			conf_val &= ~IMX_PAD_SION;
> > 154:		}
> 
> In this code snip we only see that share_conf_val is used (line 149),
> it is not assigned.
> So we do not really see the context of your message in the code snip.
> 
> 	Sam

Thank you for your feedback. I took the opportunity and had a closer
look at the code. Here is the full context of the file from before the
patch:

   83 static int imx_iomux_v3_set_state(struct pinctrl_device *pdev, struct device_node *np)
   84 {
   85         struct imx_iomux_v3 *iomux = container_of(pdev, struct imx_iomux_v3, pinctrl);
   86         const __be32 *list;
   87         const bool share_conf = iomux->flags & SHARE_CONF;
   88         int npins, size, i, fsl_pin_size;
   89         const char *name;
   90         u32 share_conf_val;
[ this is the line that was patched to say "u32 share_conf_val = 0;" ]
   91 
   92         dev_dbg(iomux->pinctrl.dev, "set state: %s\n", np->full_name);
   93 
   94         if (share_conf) {
   95                 u32 drive_strength, slew_rate;
   96                 int ret;
   97 
   98                 fsl_pin_size = SHARE_CONF_FSL_PIN_SIZE;
   99                 name = "pinmux";
  100 
  101                 ret = of_property_read_u32(np, "drive-strength",
  102                                            &drive_strength);
  103                 if (ret)
  104                         return ret;
  105 
  106                 ret = of_property_read_u32(np, "slew-rate", &slew_rate);
  107                 if (ret)
  108                         return ret;
  109 
  110                 share_conf_val =
  111                         FIELD_PREP(SHARE_CONF_PAD_CTL_DSE, drive_strength) |
  112                         FIELD_PREP(SHARE_CONF_PAD_CTL_SRE, slew_rate);
  113 
  114                 if (of_get_property(np, "drive-open-drain", NULL))
  115                         share_conf_val |= SHARE_CONF_PAD_CTL_ODE;
  116 
  117                 if (of_get_property(np, "input-schmitt-enable", NULL))
  118                         share_conf_val |= SHARE_CONF_PAD_CTL_HYS;
  119 
  120                 if (of_get_property(np, "input-enable", NULL))
  121                         share_conf_val |= IMX_PAD_SION;
  122 
  123                 if (of_get_property(np, "bias-pull-up", NULL))
  124                         share_conf_val |= SHARE_CONF_PAD_CTL_PUE;
  125         } else {
  126                 fsl_pin_size = FSL_PIN_SIZE;
  127                 name = "fsl,pins";
  128         }
  129 
  130         list = of_get_property(np, name, &size);
  131         if (!list)
  132                 return -EINVAL;
  133 
  134         if (!size || size % fsl_pin_size) {
  135                 dev_err(iomux->pinctrl.dev, "Invalid fsl,pins property in %s\n",
  136                                 np->full_name);
  137                 return -EINVAL;
  138         }
  139 
  140         npins = size / fsl_pin_size;
  141 
  142         for (i = 0; i < npins; i++) {
  143                 u32 mux_reg = be32_to_cpu(*list++);
  144                 u32 conf_reg = be32_to_cpu(*list++);
  145                 u32 input_reg = be32_to_cpu(*list++);
  146                 u32 mux_val = be32_to_cpu(*list++);
  147                 u32 input_val = be32_to_cpu(*list++);
  148                 u32 conf_val = share_conf ?
  149                         share_conf_val : be32_to_cpu(*list++);
  150 

The compiler complains because in line 148 conf_val is assigned to a
value that depends on the value of share_conf_val, which in turn is only
initialised in line 110, inside the if branch that depends on
share_conf. However, conf_val is only set to share_conf_val in the same
condition depending on share_conf, so it is guaranteed that
share_conf_val will be initialized when its value is used. I guess GCC 5
doesn't yet have a good enough heuristic to detect that case and warns
unnecessarily.

So if you feel that the (old) compiler is wrong here about the warning,
and the code itself is correct enough, feel free to leave out that patch
from the queue.

 - Roland

-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning
  2018-09-03 13:50     ` Roland Hieber
@ 2018-09-03 20:14       ` Sam Ravnborg
  2018-09-04  6:46         ` Sascha Hauer
  0 siblings, 1 reply; 9+ messages in thread
From: Sam Ravnborg @ 2018-09-03 20:14 UTC (permalink / raw)
  To: Roland Hieber; +Cc: barebox

Hi Roland.

> > 
> > In this code snip we only see that share_conf_val is used (line 149),
> > it is not assigned.
> > So we do not really see the context of your message in the code snip.
> > 
> > 	Sam
> 
> Thank you for your feedback. I took the opportunity and had a closer
> look at the code. Here is the full context of the file from before the
> patch:
> 
>    83 static int imx_iomux_v3_set_state(struct pinctrl_device *pdev, struct device_node *np)
>    84 {
     ...
>    90         u32 share_conf_val;
>    91 
     ...
>    94         if (share_conf) {
     ...
>   110                 share_conf_val =
>   111                         FIELD_PREP(SHARE_CONF_PAD_CTL_DSE, drive_strength) |
>   112                         FIELD_PREP(SHARE_CONF_PAD_CTL_SRE, slew_rate);
    ...
>   142         for (i = 0; i < npins; i++) {
    ...
>   148                 u32 conf_val = share_conf ?
>   149                         share_conf_val : be32_to_cpu(*list++);

The comment was only that despite your effort the changelog
did not provide enough context.
Above I have provided enough context to your otherwise nice explanation.

> So if you feel that the (old) compiler is wrong here about the warning,
> and the code itself is correct enough, feel free to leave out that patch
> from the queue.
The patch is IMO fine, but the changelog could be better.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning
  2018-09-03 20:14       ` Sam Ravnborg
@ 2018-09-04  6:46         ` Sascha Hauer
  0 siblings, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2018-09-04  6:46 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: barebox, Roland Hieber

On Mon, Sep 03, 2018 at 10:14:34PM +0200, Sam Ravnborg wrote:
> Hi Roland.
> 
> > > 
> > > In this code snip we only see that share_conf_val is used (line 149),
> > > it is not assigned.
> > > So we do not really see the context of your message in the code snip.
> > > 
> > > 	Sam
> > 
> > Thank you for your feedback. I took the opportunity and had a closer
> > look at the code. Here is the full context of the file from before the
> > patch:
> > 
> >    83 static int imx_iomux_v3_set_state(struct pinctrl_device *pdev, struct device_node *np)
> >    84 {
>      ...
> >    90         u32 share_conf_val;
> >    91 
>      ...
> >    94         if (share_conf) {
>      ...
> >   110                 share_conf_val =
> >   111                         FIELD_PREP(SHARE_CONF_PAD_CTL_DSE, drive_strength) |
> >   112                         FIELD_PREP(SHARE_CONF_PAD_CTL_SRE, slew_rate);
>     ...
> >   142         for (i = 0; i < npins; i++) {
>     ...
> >   148                 u32 conf_val = share_conf ?
> >   149                         share_conf_val : be32_to_cpu(*list++);
> 
> The comment was only that despite your effort the changelog
> did not provide enough context.
> Above I have provided enough context to your otherwise nice explanation.
> 
> > So if you feel that the (old) compiler is wrong here about the warning,
> > and the code itself is correct enough, feel free to leave out that patch
> > from the queue.
> The patch is IMO fine, but the changelog could be better.

Changed that to:

we have the pattern:

	if (share_conf)
		share_conf_val = ...;
	...

	if (share_conf)
		use(share_conf_val);

GCC 5.4.0 doesn't recognize this so explicitly initialize
share_conf_val.

And applied this series.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-09-04  6:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-02 21:21 [PATCH 0/4] Kindle i.MX50 improvements Roland Hieber
2018-09-02 21:21 ` [PATCH 1/4] pinctrl: imx-iomux-v3: fix compiler warning Roland Hieber
2018-09-03  4:46   ` Sam Ravnborg
2018-09-03 13:50     ` Roland Hieber
2018-09-03 20:14       ` Sam Ravnborg
2018-09-04  6:46         ` Sascha Hauer
2018-09-02 21:21 ` [PATCH 2/4] ARM: i.MX: Kindle 4/5 is based on Device Tree, select it in Kconfig Roland Hieber
2018-09-02 21:21 ` [PATCH 3/4] ARM: i.MX: add defconfig for the Kindle i.MX50 boards Roland Hieber
2018-09-02 21:21 ` [PATCH 4/4] Documentation: i.MX: update Kindle 4/5 board documentation Roland Hieber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox