diff mbox

[2/3] arm: tegra: enable igb, stmpe, i2c chardev, spidev, lm95245, pwm leds

Message ID 39a8704a4c8170d6b0620a1e5e44042eae6d8810.1401665237.git.marcel@ziswiler.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marcel Ziswiler June 1, 2014, 11:37 p.m. UTC
The NVIDIA Tegra 3 based Apalis T30 module contains an Intel i210 resp.
i211 gigabit Ethernet controller, an STMPE811 ADC/touch controller, I2C
as well as SPI buses and PWM LEDs generically accessible from user
space and an LM95245 temperature sensor chip. The later five can also
be found on the Colibri T30 module.

While at it move the PCA953x GPIO entry down to its proper place to
have it all nicely ordered again.

Signed-off-by: Marcel Ziswiler <marcel@ziswiler.com>
---
BTW: How about MTD_SPI_NOR, PROC_DEVICETREE and CRYPTO_DEV_TEGRA_AES
which I haven't found any mentioning anywhere?

 arch/arm/configs/tegra_defconfig |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Stephen Warren June 2, 2014, 4:11 p.m. UTC | #1
On 06/01/2014 05:37 PM, Marcel Ziswiler wrote:
> The NVIDIA Tegra 3 based Apalis T30 module contains an Intel i210 resp.
> i211 gigabit Ethernet controller, an STMPE811 ADC/touch controller, I2C
> as well as SPI buses and PWM LEDs generically accessible from user
> space and an LM95245 temperature sensor chip. The later five can also
> be found on the Colibri T30 module.
> 
> While at it move the PCA953x GPIO entry down to its proper place to
> have it all nicely ordered again.
> 
> Signed-off-by: Marcel Ziswiler <marcel@ziswiler.com>
> ---
> BTW: How about MTD_SPI_NOR,

That might only exist in linux-next.

> PROC_DEVICETREE and CRYPTO_DEV_TEGRA_AES
> which I haven't found any mentioning anywhere?

The TEGRA_AES driver has been removed, so the option should be removed
from defconfig too. I don't know what happened to PROC_DEVICTREE - it
doesn't seem to exist any more. Was it replaced by something else or
deleted? Feel free to send patches for those.

> diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig

> +CONFIG_SPI_SPIDEV=y

Is this useful with DT? I thought that unlike I2C_CHARDEV, spidev needed
dummy devices to exist in DT for spidev to work? If so, there's not much
point adding the option to defconfig, since people can add it when they
put the dummy devices into DT.
Marcel Ziswiler June 2, 2014, 4:28 p.m. UTC | #2
On 06/02/2014 06:11 PM, Stephen Warren wrote:
>> BTW: How about MTD_SPI_NOR,
>
> That might only exist in linux-next.
>
>> PROC_DEVICETREE and CRYPTO_DEV_TEGRA_AES
>> which I haven't found any mentioning anywhere?
>
> The TEGRA_AES driver has been removed, so the option should be removed
> from defconfig too. I don't know what happened to PROC_DEVICTREE - it
> doesn't seem to exist any more. Was it replaced by something else or
> deleted? Feel free to send patches for those.

OK, will do.

>> diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
>
>> +CONFIG_SPI_SPIDEV=y
>
> Is this useful with DT? I thought that unlike I2C_CHARDEV, spidev needed
> dummy devices to exist in DT for spidev to work? If so, there's not much
> point adding the option to defconfig, since people can add it when they
> put the dummy devices into DT.

Yes, the Apalis T30 DT I sent actually contains two of them which we 
call generic Apalis SPI1 and SPI2 out-of-the-box configured for exactly 
that. Without the config enabled though it probably does not make much 
sense to include it in the DT so I would consider removing it again.
Mark Brown June 2, 2014, 10:16 p.m. UTC | #3
On Mon, Jun 02, 2014 at 06:28:37PM +0200, Marcel Ziswiler wrote:
> On 06/02/2014 06:11 PM, Stephen Warren wrote:

> >>+CONFIG_SPI_SPIDEV=y

> >Is this useful with DT? I thought that unlike I2C_CHARDEV, spidev needed
> >dummy devices to exist in DT for spidev to work? If so, there's not much
> >point adding the option to defconfig, since people can add it when they
> >put the dummy devices into DT.

> Yes, the Apalis T30 DT I sent actually contains two of them which we call
> generic Apalis SPI1 and SPI2 out-of-the-box configured for exactly that.
> Without the config enabled though it probably does not make much sense to
> include it in the DT so I would consider removing it again.

Your DT is broken if it's got a "spidev" node in it, you should be
describing the hardware not the Linux implementation of the software.
It would be really nice if we had a good way of handling this but we
don't yet.
Marcel Ziswiler June 3, 2014, 6:02 a.m. UTC | #4
On 06/03/2014 12:16 AM, Mark Brown wrote:
> On Mon, Jun 02, 2014 at 06:28:37PM +0200, Marcel Ziswiler wrote:
>> On 06/02/2014 06:11 PM, Stephen Warren wrote:
>
>>>> +CONFIG_SPI_SPIDEV=y
>
>>> Is this useful with DT? I thought that unlike I2C_CHARDEV, spidev needed
>>> dummy devices to exist in DT for spidev to work? If so, there's not much
>>> point adding the option to defconfig, since people can add it when they
>>> put the dummy devices into DT.
>
>> Yes, the Apalis T30 DT I sent actually contains two of them which we call
>> generic Apalis SPI1 and SPI2 out-of-the-box configured for exactly that.
>> Without the config enabled though it probably does not make much sense to
>> include it in the DT so I would consider removing it again.
>
> Your DT is broken if it's got a "spidev" node in it, you should be
> describing the hardware not the Linux implementation of the software.
> It would be really nice if we had a good way of handling this but we
> don't yet.

I strongly disagree, it almost perfectly describes the hardware. Unlike 
on I2c where modelling a bus is enough to allow generic user space 
access unfortunately on SPI this is not enough as it requires a specific 
chip-select as well. This is exactly what spidev does and maps to our 
hardware perfectly which has one dedicated chip-select per SPI bus on a 
dedicated header which allows our customers out-of-the-box spidev user 
space access to almost any SPI device connected to those buses just like 
with i2c-devs on I2C buses.
Mark Brown June 3, 2014, 9:45 a.m. UTC | #5
On Tue, Jun 03, 2014 at 08:02:37AM +0200, Marcel Ziswiler wrote:
> On 06/03/2014 12:16 AM, Mark Brown wrote:

> >Your DT is broken if it's got a "spidev" node in it, you should be
> >describing the hardware not the Linux implementation of the software.
> >It would be really nice if we had a good way of handling this but we
> >don't yet.

> I strongly disagree, it almost perfectly describes the hardware. Unlike on
> I2c where modelling a bus is enough to allow generic user space access
> unfortunately on SPI this is not enough as it requires a specific
> chip-select as well. This is exactly what spidev does and maps to our
> hardware perfectly which has one dedicated chip-select per SPI bus on a
> dedicated header which allows our customers out-of-the-box spidev user space
> access to almost any SPI device connected to those buses just like with
> i2c-devs on I2C buses.

When you say "generic user space access" you are describing a specific
detail of how this device happens to be controlled with your software.
This is not a description of your hardware, it is a description of how
it is controlled with your current software.
Marcel Ziswiler June 4, 2014, 6:20 a.m. UTC | #6
On 06/03/2014 11:45 AM, Mark Brown wrote:
> When you say "generic user space access" you are describing a specific
> detail of how this device happens to be controlled with your software.

No, not at all. In fact I did not even specify neither the exact type of 
device apart from it being a SPI device nor any property of the software 
apart from the generic user space access thereof implemented in the 
Linux kernel. I really don't see any difference to i2c chardev which is 
already enabled in multi_v7_defconfig.

> This is not a description of your hardware, it is a description of how
> it is controlled with your current software.

Sorry, but I really don't know what you are referring to. It's a pure 
hardware description of some pins being the SPI bus namely MISO/MOSI and 
the clock plus an accompanying chip-select pin.

I fear for some reason or another you have some affinity against spidev 
which strikes me odd. Admittedly it is not perfect but it is the only 
generic SPI user space access currently implemented in the Linux kernel 
and so far did its job perfectly for many of our customers.
Mark Brown June 4, 2014, 11:17 a.m. UTC | #7
On Wed, Jun 04, 2014 at 08:20:59AM +0200, Marcel Ziswiler wrote:
> On 06/03/2014 11:45 AM, Mark Brown wrote:

> >When you say "generic user space access" you are describing a specific
> >detail of how this device happens to be controlled with your software.

> No, not at all. In fact I did not even specify neither the exact type of
> device apart from it being a SPI device nor any property of the software
> apart from the generic user space access thereof implemented in the Linux

You're saying you're controlling it from userspace.  This is a
particular detail of what you are doing in your system.  You happen to
want to control the devices you are hanging off the system with
userspace drivers but that's just what you're doing right now.

> kernel. I really don't see any difference to i2c chardev which is already
> enabled in multi_v7_defconfig.

That does not require explicit registration as the device driver for the
device in order to be used.

> >This is not a description of your hardware, it is a description of how
> >it is controlled with your current software.

> Sorry, but I really don't know what you are referring to. It's a pure
> hardware description of some pins being the SPI bus namely MISO/MOSI and the
> clock plus an accompanying chip-select pin.

No, that's in the controller node - the chip selects are described
there.  The child node references a chip select number that the master
has and describes what's connected to that chip select.

> I fear for some reason or another you have some affinity against spidev
> which strikes me odd. Admittedly it is not perfect but it is the only
> generic SPI user space access currently implemented in the Linux kernel and
> so far did its job perfectly for many of our customers.

It's a perfectly fine way of controlling things from userspace if that's
a sensible way of controlling devices but that does not mean you should
describe it in the device tree in that fashion.
Marcel Ziswiler June 9, 2014, 10:16 p.m. UTC | #8
On 06/04/2014 01:17 PM, Mark Brown wrote:
> You're saying you're controlling it from userspace.  This is a
> particular detail of what you are doing in your system.  You happen to
> want to control the devices you are hanging off the system with
> userspace drivers but that's just what you're doing right now.

Sorry, I don't get it. Yes, spidev is to control stuff from user space 
just like i2c-dev however bad that might sound.

> No, that's in the controller node - the chip selects are described
> there.  The child node references a chip select number that the master
> has and describes what's connected to that chip select.

Well, unfortunately SPI without any chip select is just plain simply 
useless. It won't work.

> It's a perfectly fine way of controlling things from userspace if that's
> a sensible way of controlling devices but that does not mean you should
> describe it in the device tree in that fashion.

Only that without describing such a chip select in the device tree 
spidev won't ever work.

I don't see us reaching any consensus here therefore I retreat. I will 
re-submit the whole thing without spidev however sad having to see that 
useful feature being dropped.
Mark Brown June 9, 2014, 10:57 p.m. UTC | #9
On Tue, Jun 10, 2014 at 12:16:13AM +0200, Marcel Ziswiler wrote:
> On 06/04/2014 01:17 PM, Mark Brown wrote:

> >You're saying you're controlling it from userspace.  This is a
> >particular detail of what you are doing in your system.  You happen to
> >want to control the devices you are hanging off the system with
> >userspace drivers but that's just what you're doing right now.

> Sorry, I don't get it. Yes, spidev is to control stuff from user space just
> like i2c-dev however bad that might sound.

There is absolutely nothing wrong with that.  What there is a problem
with is putting that implementation detail into a device tree, if
someone comes along and writes an in kernel driver for the same device
the device tree needs to continue to work without modification as it is
an ABI.

> >No, that's in the controller node - the chip selects are described
> >there.  The child node references a chip select number that the master
> >has and describes what's connected to that chip select.

> Well, unfortunately SPI without any chip select is just plain simply
> useless. It won't work.

I'm sorry but I'm not seeing how that follows on from what I said?

> >It's a perfectly fine way of controlling things from userspace if that's
> >a sensible way of controlling devices but that does not mean you should
> >describe it in the device tree in that fashion.

> Only that without describing such a chip select in the device tree spidev
> won't ever work.

Again I'm not sure how that follows.  To repeat, the chip selects are
described on the SPI controller and referenced by the child devices when
they are instantiated by chip select number.  Please refer to the SPI
bindings if this is unclear, or be specific in what is unclear.
diff mbox

Patch

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index fb25e29..4ed82f9 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -23,7 +23,6 @@  CONFIG_MODULE_FORCE_UNLOAD=y
 CONFIG_PARTITION_ADVANCED=y
 # CONFIG_IOSCHED_DEADLINE is not set
 # CONFIG_IOSCHED_CFQ is not set
-CONFIG_GPIO_PCA953X=y
 CONFIG_ARCH_TEGRA=y
 CONFIG_ARCH_TEGRA_2x_SOC=y
 CONFIG_ARCH_TEGRA_3x_SOC=y
@@ -111,6 +110,7 @@  CONFIG_SCSI_MULTI_LUN=y
 # CONFIG_SCSI_LOWLEVEL is not set
 CONFIG_NETDEVICES=y
 CONFIG_DUMMY=y
+CONFIG_IGB=y
 CONFIG_R8169=y
 CONFIG_USB_PEGASUS=y
 CONFIG_USB_USBNET=y
@@ -125,6 +125,8 @@  CONFIG_KEYBOARD_GPIO=y
 CONFIG_KEYBOARD_TEGRA=y
 CONFIG_KEYBOARD_CROS_EC=y
 CONFIG_MOUSE_PS2_ELANTECH=y
+CONFIG_INPUT_TOUCHSCREEN=y
+CONFIG_TOUCHSCREEN_STMPE=y
 CONFIG_INPUT_MISC=y
 CONFIG_INPUT_MPU3050=y
 # CONFIG_LEGACY_PTYS is not set
@@ -135,6 +137,7 @@  CONFIG_SERIAL_TEGRA=y
 CONFIG_SERIAL_OF_PLATFORM=y
 # CONFIG_HW_RANDOM is not set
 # CONFIG_I2C_COMPAT is not set
+CONFIG_I2C_CHARDEV=y
 CONFIG_I2C_MUX_PCA954x=y
 CONFIG_I2C_MUX_PINCTRL=y
 CONFIG_I2C_TEGRA=y
@@ -142,8 +145,10 @@  CONFIG_SPI=y
 CONFIG_SPI_TEGRA114=y
 CONFIG_SPI_TEGRA20_SFLASH=y
 CONFIG_SPI_TEGRA20_SLINK=y
+CONFIG_SPI_SPIDEV=y
 CONFIG_PINCTRL_AS3722=y
 CONFIG_PINCTRL_PALMAS=y
+CONFIG_GPIO_PCA953X=y
 CONFIG_GPIO_PCA953X_IRQ=y
 CONFIG_GPIO_PALMAS=y
 CONFIG_GPIO_TPS6586X=y
@@ -155,10 +160,12 @@  CONFIG_POWER_RESET=y
 CONFIG_POWER_RESET_AS3722=y
 CONFIG_POWER_RESET_GPIO=y
 CONFIG_SENSORS_LM90=y
+CONFIG_SENSORS_LM95245=y
 CONFIG_MFD_AS3722=y
 CONFIG_MFD_CROS_EC=y
 CONFIG_MFD_CROS_EC_SPI=y
 CONFIG_MFD_MAX8907=y
+CONFIG_MFD_STMPE=y
 CONFIG_MFD_PALMAS=y
 CONFIG_MFD_TPS65090=y
 CONFIG_MFD_TPS6586X=y
@@ -221,6 +228,7 @@  CONFIG_MMC_SDHCI_TEGRA=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_GPIO=y
+CONFIG_LEDS_PWM=y
 CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_TIMER=y
 CONFIG_LEDS_TRIGGER_ONESHOT=y