All of lore.kernel.org
 help / color / mirror / Atom feed
From: nsekhar@ti.com (Sekhar Nori)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/4] ARM: davinci: make I2C support optional
Date: Fri, 5 Feb 2016 18:50:29 +0530	[thread overview]
Message-ID: <56B4A19D.4060307@ti.com> (raw)
In-Reply-To: <1454358962-640598-5-git-send-email-arnd@arndb.de>

On Tuesday 02 February 2016 02:05 AM, Arnd Bergmann wrote:
> The davinci platform has tried to get support for the EEPROM right,
> but failed to get a clean build so far. At the moment, we get
> a warning whenever CONFIG_SYSFS is disabled, as that is needed by
> EEPROM_AT24:
> 
> warning: (MACH_DAVINCI_EVM && MACH_SFFSDR && MACH_DAVINCI_DM6467_EVM && MACH_DAVINCI_DM365_EVM && MACH_DAVINCI_DA830_EVM && MACH_MITYOMAPL138 && MACH_MINI2440) selects EEPROM_AT24 which has unmet direct dependencies (I2C && SYSFS)
> 
> Kevin Hilman initially added the 'select' to ensure that EEPROM_AT24
> is always enabled in machines that really want it for normal operation
> (i.e. for reading the MAC address). This broke when I2C was disabled,
> and Russell King followed up with another patch to select that as
> well.
> 
> I now see that the SYSFS dependency is still missing, which leaves
> us with three options:
> 
> a) add 'select SYSFS' in addition to the others
> b) change AT24_EEPPROM to work without sysfs (should be possible)
> c) remove all those selects again and get the files to build when
>    I2C is disabled.
> 
> I would really hate to do a) because adding select statements that
> hardwire user-selectable symbols is generally a bad idea. I first
> tried b) but then ended up redoing the patch from scratch to approach
> c), so we can also remove the other selects.
> 
> I checked that CONFIG_I2C is still enabled with davinci_all_defconfig,
> so that does not have to change.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 45b146d746ea ("ARM: Davinci: Fix I2C build errors")
> Fixes: 22ca466847ad ("davinci: kconfig: select at24 eeprom for selected boards")

This looks good to me. The #ifdefs in the middle of davinci_evm_init()
are an eyesore, but getting rid of the selects is a big plus.

Thanks,
Sekhar

WARNING: multiple messages have this Message-ID (diff)
From: Sekhar Nori <nsekhar@ti.com>
To: Arnd Bergmann <arnd@arndb.de>,
	Kevin Hilman <khilman@deeprootsystems.com>
Cc: <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/4] ARM: davinci: make I2C support optional
Date: Fri, 5 Feb 2016 18:50:29 +0530	[thread overview]
Message-ID: <56B4A19D.4060307@ti.com> (raw)
In-Reply-To: <1454358962-640598-5-git-send-email-arnd@arndb.de>

On Tuesday 02 February 2016 02:05 AM, Arnd Bergmann wrote:
> The davinci platform has tried to get support for the EEPROM right,
> but failed to get a clean build so far. At the moment, we get
> a warning whenever CONFIG_SYSFS is disabled, as that is needed by
> EEPROM_AT24:
> 
> warning: (MACH_DAVINCI_EVM && MACH_SFFSDR && MACH_DAVINCI_DM6467_EVM && MACH_DAVINCI_DM365_EVM && MACH_DAVINCI_DA830_EVM && MACH_MITYOMAPL138 && MACH_MINI2440) selects EEPROM_AT24 which has unmet direct dependencies (I2C && SYSFS)
> 
> Kevin Hilman initially added the 'select' to ensure that EEPROM_AT24
> is always enabled in machines that really want it for normal operation
> (i.e. for reading the MAC address). This broke when I2C was disabled,
> and Russell King followed up with another patch to select that as
> well.
> 
> I now see that the SYSFS dependency is still missing, which leaves
> us with three options:
> 
> a) add 'select SYSFS' in addition to the others
> b) change AT24_EEPPROM to work without sysfs (should be possible)
> c) remove all those selects again and get the files to build when
>    I2C is disabled.
> 
> I would really hate to do a) because adding select statements that
> hardwire user-selectable symbols is generally a bad idea. I first
> tried b) but then ended up redoing the patch from scratch to approach
> c), so we can also remove the other selects.
> 
> I checked that CONFIG_I2C is still enabled with davinci_all_defconfig,
> so that does not have to change.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 45b146d746ea ("ARM: Davinci: Fix I2C build errors")
> Fixes: 22ca466847ad ("davinci: kconfig: select at24 eeprom for selected boards")

This looks good to me. The #ifdefs in the middle of davinci_evm_init()
are an eyesore, but getting rid of the selects is a big plus.

Thanks,
Sekhar

  reply	other threads:[~2016-02-05 13:20 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-01 20:35 [PATCH 0/4] ARM Davinci warning fixes Arnd Bergmann
2016-02-01 20:35 ` [PATCH 1/4] ARM: davinci: limit DT support to DA850 Arnd Bergmann
2016-02-01 20:35   ` Arnd Bergmann
2016-02-01 20:35 ` [PATCH 2/4] ARM: davinci: avoid unused mityomapl138_pn_info variable Arnd Bergmann
2016-02-01 20:35   ` Arnd Bergmann
2016-02-01 20:35 ` [PATCH 3/4] ARM: davinci: DA8xx+DMx combined kernels need PATCH_PHYS_VIRT Arnd Bergmann
2016-02-01 20:35   ` Arnd Bergmann
2016-02-01 20:35 ` [PATCH 4/4] ARM: davinci: make I2C support optional Arnd Bergmann
2016-02-01 20:35   ` Arnd Bergmann
2016-02-05 13:20   ` Sekhar Nori [this message]
2016-02-05 13:20     ` Sekhar Nori
2016-02-05 14:15     ` Arnd Bergmann
2016-02-05 14:15       ` Arnd Bergmann
2016-02-17 15:11 ` [PATCH 0/4] ARM Davinci warning fixes Sekhar Nori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56B4A19D.4060307@ti.com \
    --to=nsekhar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.