[linux dev-4.19 00/11] Add NPCM7xx patches to dev-4.19

Joel Stanley joel at jms.id.au
Thu Jan 3 17:11:18 AEDT 2019


Hi Tomer,

On Sun, 16 Dec 2018 at 21:30, Tomer Maimon <tmaimon77 at gmail.com> wrote:
>
> Add the following NPCM7xx patches to dev-4.19:

I had some email issues where not all of these patches arrived in my
inbox. They are on patchwork, so I was able to review them.

>
> 1. NPCM7xx FIU driver(SPI-NOR).
> 2. NPCM7xx I2C driver.

The i2c does not build, so I did not include that patch:

../drivers/i2c/busses/i2c-npcm7xx.c:9:10: fatal error:
linux/clk/nuvoton.h: No such file or directory
 #include <linux/clk/nuvoton.h>
          ^~~~~~~~~~~~~~~~~~~~~

Please re-send it once you've tested that it applies and builds. It
would be great if you had also tested it on hardware before sending.

> 3. NPCM7xx BIOS post code driver.

This introduces a warning:

../drivers/misc/npcm7xx-lpc-bpc.c: In function ‘npcm7xx_bpc_irq’:
../drivers/misc/npcm7xx-lpc-bpc.c:150:25: warning: ‘last_addr_bit’ may
be used uninitialized in this function [-Wmaybe-uninitialized]
   if (lpc_bpc->en_dwcap && last_addr_bit) {
       ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~

I would strongly encourage you to look at the aspeed snoop driver and
adopt a common userspace interface for this code. It's important to do
that before userspace code is written that depends on this interface.

> 4. NPCM7xx PCI mailbox driver.

I didn't merge this one has it contained a bad size check. Please take
a look at npcm7xx_mbox_write:

+       if (count + *ppos > mbox->max_buf_size)
+               return -EINVAL;

If you resend just the mailbox driver (and bindings) I will merge them
separately.

> 5. NPCM7xx ETHERNET MAC Controller driver.
> 6. NPCM7xx defconfig.

I found some warnings and errors when building your patches

The device tree updates cause this warning when building:

  DTC     arch/arm/boot/dts/nuvoton-npcm750-evb.dtb
arch/arm/boot/dts/nuvoton-npcm750-evb.dtb: Warning (pci_bridge):
/ahb/axi-pcie at e1000000: node name is not "pci" or "pcie"

The ADC driver causes this warning:

../drivers/iio/adc/npcm_adc.c: In function ‘npcm_adc_probe’:
../drivers/iio/adc/npcm_adc.c:301:9: warning: ‘ret’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
  return ret;
         ^~~

I took a look at the driver and it is clear it needs some further work. eg:

                if (PTR_ERR(info->vref) != -ENODEV) {
                        goto err_disable_clk;
                        return PTR_ERR(info->vref);
                }

and:

        if (of_device_is_compatible(np, "nuvoton,npcm750-adc")) {

Can you please take another look at the series before we merge it?
Here's a checklist:

1. Build the code
2. Check for warnings
3. Run the patches through checkpatch.pl

Thanks,

Joel


More information about the openbmc mailing list