Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1479900

Re: [PATCH v3 2/3] intel-mid: Add valid error messages on init failure

From Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 2/3] intel-mid: Add valid error messages on init failure
Date 2016-09-09 13:30 +0200
Message-ID <sfsam-2aJ-21@gated-at.bofh.it> (permalink)
References <sezxf-1jF-1@gated-at.bofh.it> <sfjA5-5sJ-5@gated-at.bofh.it>
Organization Intel Finland Oy

Show all headers | View raw


On Thu, 2016-09-08 at 19:07 -0700, Kuppuswamy Sathyanarayanan wrote:
> Added valid error/warning messages to platform data
> initalization failures in SFI device libs code.

Looks good to me after addressing the following comments.

> 
> diff --git a/arch/x86/platform/intel-
> mid/device_libs/platform_emc1403.c b/arch/x86/platform/intel-
> mid/device_libs/platform_emc1403.c
> index c259fb6..bd776b04 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_emc1403.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_emc1403.c
> @@ -15,17 +15,27 @@
>  #include <linux/i2c.h>
>  #include <asm/intel-mid.h>
>  
> +#define EMC1403_THERMAL_INT		"thermal_int"
> +#define EMC1403_THERMAL_ALERT_INT	"thermal_alert"

I would be cosistent, i.e.
EMC1403_INT1  "..."
EMC1403_INT2  "..."

> +
>  static void __init *emc1403_platform_data(void *info)
>  {
>  	static short intr2nd_pdata;
>  	struct i2c_board_info *i2c_info = info;
> -	int intr = get_gpio_by_name("thermal_int");
> -	int intr2nd = get_gpio_by_name("thermal_alert");
> +	int intr = get_gpio_by_name(EMC1403_THERMAL_INT);
> +	int intr2nd = get_gpio_by_name(EMC1403_THERMAL_ALERT_INT);
>  
> -	if (intr < 0)
> +	if (intr < 0) {
> +		pr_err("%s: Can't find %s GPIO interrupt\n",
> __func__,
> +		       EMC1403_THERMAL_INT);
>  		return NULL;

Souldn't we return an error here?

> -	if (intr2nd < 0)
> +	}
> +
> +	if (intr2nd < 0) {
> +		pr_err("%s: Can't find %s GPIO interrupt\n",
> __func__,
> +		       EMC1403_THERMAL_ALERT_INT);
>  		return NULL;

Ditto.

Would you check _all_ files under device libs?

> +	}
>  
>  	i2c_info->irq = intr + INTEL_MID_IRQ_OFFSET;
>  	intr2nd_pdata = intr2nd + INTEL_MID_IRQ_OFFSET;
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_ipc.c
> b/arch/x86/platform/intel-mid/device_libs/platform_ipc.c
> index a84b73d..6704694 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_ipc.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_ipc.c
> @@ -42,8 +42,11 @@ void __init ipc_device_handler(struct
> sfi_device_table_entry *pentry,
>  	 * On Medfield the platform device creation is handled by the
> MSIC
>  	 * MFD driver so we don't need to do it here.
>  	 */
> -	if (intel_mid_has_msic())
> +	if (intel_mid_has_msic()) {
> +		pr_err("%s: device %s will be handled by MSIC mfd
> driver\n",

Remove "mfd" word.

> +		       __func__, pentry->name);
>  		return;
> +	}
>  
>  	pdev = platform_device_alloc(pentry->name, 0);
>  	if (pdev == NULL) {
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_lis331.c 
> b/arch/x86/platform/intel-mid/device_libs/platform_lis331.c
> index 8be5d40..393c23e 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_lis331.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_lis331.c
> @@ -14,17 +14,27 @@
>  #include <linux/gpio.h>
>  #include <asm/intel-mid.h>
>  
> +#define LIS331DL_ACCEL_INT1	"accel_int"
> +#define LIS331DL_ACCEL_INT2	"accel_2"

LIS331DL_INT1
LIS331DL_INT2

> @@ -48,8 +48,12 @@ static void __init *max7315_platform_data(void
> *info)
>  	gpio_base = get_gpio_by_name(base_pin_name);
>  	intr = get_gpio_by_name(intr_pin_name);
>  
> -	if (gpio_base < 0)
> +	if (gpio_base < 0) {
> +		pr_warn("%s: falling back to dynamic gpio
> allocation\n",
> +			__func__);
>  		return NULL;

Depends on my comment to the previous series.

> --- a/arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c
> @@ -14,13 +14,18 @@
>  #include <linux/i2c.h>
>  #include <asm/intel-mid.h>
>  
> +#define MPU3050_INT		"mpu3050_int"

MPU3050_INT1

> mid/device_libs/platform_pcal9555a.c b/arch/x86/platform/intel-
> mid/device_libs/platform_pcal9555a.c
> index 563f77f..cde764e 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_pcal9555a.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_pcal9555a.c
> @@ -41,8 +41,11 @@ static void __init *pcal9555a_platform_data(void
> *info)
>  	intr = get_gpio_by_name(intr_pin_name);
>  
>  	/* Check if the SFI record valid */
> -	if (gpio_base == -1)
> +	if (gpio_base == -1) {
> +		pr_warn("%s: falling back to dynamic gpio
> allocation\n",
> +			__func__);
>  		return NULL;

Same as above

> mid/device_libs/platform_tca6416.c b/arch/x86/platform/intel-
> mid/device_libs/platform_tca6416.c
> index 4f41372..4d4393e 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_tca6416.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_tca6416.c
> @@ -34,8 +34,12 @@ static void *tca6416_platform_data(void *info)
>  	gpio_base = get_gpio_by_name(base_pin_name);
>  	intr = get_gpio_by_name(intr_pin_name);
>  
> -	if (gpio_base < 0)
> +	if (gpio_base < 0) {
> +		pr_warn("%s: falling back to dynamic gpio
> allocation\n",
> +			__func__);
>  		return NULL;

Same as above

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread


Thread

[PATCH 1/1] intel-mid: Fix sfi get_platform_data() return value issues Kuppuswamy Sathyanarayanan          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-07 03:10 +0200
  Re: [PATCH 1/1] intel-mid: Fix sfi get_platform_data() return value  issues Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-07 14:20 +0200
    Re: [PATCH 1/1] intel-mid: Fix sfi get_platform_data() return value  issues sathyanarayanan kuppuswamy          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-08 02:10 +0200
      Re: [PATCH 1/1] intel-mid: Fix sfi get_platform_data() return value  issues Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-08 11:50 +0200
  [PATCH v2 1/1] intel-mid: Fix sfi get_platform_data() return value issues Kuppuswamy Sathyanarayanan          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-08 02:10 +0200
    Re: [PATCH v2 1/1] intel-mid: Fix sfi get_platform_data() return  value issues Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-08 15:00 +0200
      Re: [PATCH v2 1/1] intel-mid: Fix sfi get_platform_data() return  value issues sathyanarayanan kuppuswamy          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-09 00:50 +0200
        Re: [PATCH v2 1/1] intel-mid: Fix sfi get_platform_data() return  value issues Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-09 13:30 +0200
  [PATCH v3 3/3] intel-mid: Move boundry check to the start of init code Kuppuswamy Sathyanarayanan          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-09 04:20 +0200
    Re: [PATCH v3 3/3] intel-mid: Move boundry check to the start of  init code Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-09 13:40 +0200
  [PATCH v3 1/3] intel-mid: Fix sfi get_platform_data() return value issues Kuppuswamy Sathyanarayanan          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-09 04:20 +0200
  [PATCH v3 2/3] intel-mid: Add valid error messages on init failure Kuppuswamy Sathyanarayanan          <sathyanarayanan.kuppuswamy@linux.intel.com> - 2016-09-09 04:20 +0200
    Re: [PATCH v3 2/3] intel-mid: Add valid error messages on init  failure Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-09-09 13:30 +0200

csiph-web