| Message ID | 1326394818-32227-7-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 01/12/2012 12:00 PM, Simon Glass wrote: > Select the port ordering for I2C on Seaboard. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Acked-by: Stephen Warren <swarren@nvidia.com> This isn't the patch that I ack'd. > diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not tegra2-seaboard.dts to match the naming in the kernel? > @@ -44,4 +49,9 @@ > usb@c5004000 { > status = "disabled"; > }; > + > + i2c@7000c400 { > + status = "disabled"; > + }; > + > }; That chunk wasn't in the original patch, and doesn't match the kernel's .dts file (and I believe that I2C controller really is in use, so shouldn't be disabled).
Hi Stephen, On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: > On 01/12/2012 12:00 PM, Simon Glass wrote: >> Select the port ordering for I2C on Seaboard. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> Acked-by: Stephen Warren <swarren@nvidia.com> > > This isn't the patch that I ack'd. Sorry, I added the disable. > >> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts > > Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not > tegra2-seaboard.dts to match the naming in the kernel? > >> @@ -44,4 +49,9 @@ >> usb@c5004000 { >> status = "disabled"; >> }; >> + >> + i2c@7000c400 { >> + status = "disabled"; >> + }; >> + >> }; > > That chunk wasn't in the original patch, and doesn't match the kernel's > .dts file (and I believe that I2C controller really is in use, so > shouldn't be disabled). It cannot be used - remember the discussion about pinmux? We elected to disable I2C1 at present since you didn't like my nvidia,pinmux binding for selecting which value to pass to funcmux. The fix is to pass 1 instead of 0 for that port, but we have no clean way to specify this. Rather disable it than leave it enabled and not working. Regards, Simon > > -- > nvpublic
On 02/03/2012 04:24 PM, Simon Glass wrote: > Hi Stephen, > > On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: >> On 01/12/2012 12:00 PM, Simon Glass wrote: >>> Select the port ordering for I2C on Seaboard. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> Acked-by: Stephen Warren <swarren@nvidia.com> >> >> This isn't the patch that I ack'd. > > Sorry, I added the disable. > >> >>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts >> >> Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not >> tegra2-seaboard.dts to match the naming in the kernel? >> >>> @@ -44,4 +49,9 @@ >>> usb@c5004000 { >>> status = "disabled"; >>> }; >>> + >>> + i2c@7000c400 { >>> + status = "disabled"; >>> + }; >>> + >>> }; >> >> That chunk wasn't in the original patch, and doesn't match the kernel's >> .dts file (and I believe that I2C controller really is in use, so >> shouldn't be disabled). > > It cannot be used - remember the discussion about pinmux? We elected > to disable I2C1 at present since you didn't like my nvidia,pinmux > binding for selecting which value to pass to funcmux. The fix is to > pass 1 instead of 0 for that port, but we have no clean way to specify > this. > > Rather disable it than leave it enabled and not working. Rather than having the .dts file not correctly describe the HW, wouldn't it be better to limit U-Boot's to only initializing the 1 I2C controller that it knows the valid pinmux setting for?
Hi Stephen, On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren <swarren@nvidia.com> wrote: > On 02/03/2012 04:24 PM, Simon Glass wrote: >> Hi Stephen, >> >> On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: >>> On 01/12/2012 12:00 PM, Simon Glass wrote: >>>> Select the port ordering for I2C on Seaboard. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> Acked-by: Stephen Warren <swarren@nvidia.com> >>> >>> This isn't the patch that I ack'd. >> >> Sorry, I added the disable. >> >>> >>>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts >>> >>> Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not >>> tegra2-seaboard.dts to match the naming in the kernel? >>> >>>> @@ -44,4 +49,9 @@ >>>> usb@c5004000 { >>>> status = "disabled"; >>>> }; >>>> + >>>> + i2c@7000c400 { >>>> + status = "disabled"; >>>> + }; >>>> + >>>> }; >>> >>> That chunk wasn't in the original patch, and doesn't match the kernel's >>> .dts file (and I believe that I2C controller really is in use, so >>> shouldn't be disabled). >> >> It cannot be used - remember the discussion about pinmux? We elected >> to disable I2C1 at present since you didn't like my nvidia,pinmux >> binding for selecting which value to pass to funcmux. The fix is to >> pass 1 instead of 0 for that port, but we have no clean way to specify >> this. >> >> Rather disable it than leave it enabled and not working. > > Rather than having the .dts file not correctly describe the HW, wouldn't > it be better to limit U-Boot's to only initializing the 1 I2C controller > that it knows the valid pinmux setting for? Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not used or connected to anything, so it is a reasonable description of the hardware. The other 3 pinmux settings are valid. Regards, Simon > > -- > nvpublic
On 02/03/2012 05:19 PM, Simon Glass wrote: > Hi Stephen, > > On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren <swarren@nvidia.com> wrote: >> On 02/03/2012 04:24 PM, Simon Glass wrote: >>> Hi Stephen, >>> >>> On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: >>>> On 01/12/2012 12:00 PM, Simon Glass wrote: >>>>> Select the port ordering for I2C on Seaboard. >>>>> >>>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>>> Acked-by: Stephen Warren <swarren@nvidia.com> >>>> >>>> This isn't the patch that I ack'd. >>> >>> Sorry, I added the disable. >>> >>>> >>>>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts >>>> >>>> Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not >>>> tegra2-seaboard.dts to match the naming in the kernel? >>>> >>>>> @@ -44,4 +49,9 @@ >>>>> usb@c5004000 { >>>>> status = "disabled"; >>>>> }; >>>>> + >>>>> + i2c@7000c400 { >>>>> + status = "disabled"; >>>>> + }; >>>>> + >>>>> }; >>>> >>>> That chunk wasn't in the original patch, and doesn't match the kernel's >>>> .dts file (and I believe that I2C controller really is in use, so >>>> shouldn't be disabled). >>> >>> It cannot be used - remember the discussion about pinmux? We elected >>> to disable I2C1 at present since you didn't like my nvidia,pinmux >>> binding for selecting which value to pass to funcmux. The fix is to >>> pass 1 instead of 0 for that port, but we have no clean way to specify >>> this. >>> >>> Rather disable it than leave it enabled and not working. >> >> Rather than having the .dts file not correctly describe the HW, wouldn't >> it be better to limit U-Boot's to only initializing the 1 I2C controller >> that it knows the valid pinmux setting for? > > Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not > used or connected to anything, so it is a reasonable description of > the hardware. The other 3 pinmux settings are valid. As best I can tell from my schematics, all 4 I2C ports are used on both Seaboard and Springbank.
Hi Stephen, On Fri, Feb 3, 2012 at 4:25 PM, Stephen Warren <swarren@nvidia.com> wrote: > On 02/03/2012 05:19 PM, Simon Glass wrote: >> Hi Stephen, >> >> On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren <swarren@nvidia.com> wrote: >>> On 02/03/2012 04:24 PM, Simon Glass wrote: >>>> Hi Stephen, >>>> >>>> On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: >>>>> On 01/12/2012 12:00 PM, Simon Glass wrote: >>>>>> Select the port ordering for I2C on Seaboard. >>>>>> >>>>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>>>> Acked-by: Stephen Warren <swarren@nvidia.com> >>>>> >>>>> This isn't the patch that I ack'd. >>>> >>>> Sorry, I added the disable. >>>> >>>>> >>>>>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts >>>>> >>>>> Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not >>>>> tegra2-seaboard.dts to match the naming in the kernel? >>>>> >>>>>> @@ -44,4 +49,9 @@ >>>>>> usb@c5004000 { >>>>>> status = "disabled"; >>>>>> }; >>>>>> + >>>>>> + i2c@7000c400 { >>>>>> + status = "disabled"; >>>>>> + }; >>>>>> + >>>>>> }; >>>>> >>>>> That chunk wasn't in the original patch, and doesn't match the kernel's >>>>> .dts file (and I believe that I2C controller really is in use, so >>>>> shouldn't be disabled). >>>> >>>> It cannot be used - remember the discussion about pinmux? We elected >>>> to disable I2C1 at present since you didn't like my nvidia,pinmux >>>> binding for selecting which value to pass to funcmux. The fix is to >>>> pass 1 instead of 0 for that port, but we have no clean way to specify >>>> this. >>>> >>>> Rather disable it than leave it enabled and not working. >>> >>> Rather than having the .dts file not correctly describe the HW, wouldn't >>> it be better to limit U-Boot's to only initializing the 1 I2C controller >>> that it knows the valid pinmux setting for? >> >> Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not >> used or connected to anything, so it is a reasonable description of >> the hardware. The other 3 pinmux settings are valid. > > As best I can tell from my schematics, all 4 I2C ports are used on both > Seaboard and Springbank. I don't know about Springbank, but this patch is for Seaboard. There is nothing on the bus and I can't see anything on the schematic. Can you please tell me what it is used for and which pins you are referring to? Regards, Simon > > -- > nvpublic
On 02/03/2012 05:36 PM, Simon Glass wrote: > Hi Stephen, > > On Fri, Feb 3, 2012 at 4:25 PM, Stephen Warren <swarren@nvidia.com> wrote: >> On 02/03/2012 05:19 PM, Simon Glass wrote: >>> Hi Stephen, >>> >>> On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren <swarren@nvidia.com> wrote: >>>> On 02/03/2012 04:24 PM, Simon Glass wrote: >>>>> Hi Stephen, >>>>> >>>>> On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: >>>>>> On 01/12/2012 12:00 PM, Simon Glass wrote: >>>>>>> Select the port ordering for I2C on Seaboard. >>>>>>> >>>>>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>>>>> Acked-by: Stephen Warren <swarren@nvidia.com> >>>>>> >>>>>> This isn't the patch that I ack'd. >>>>> >>>>> Sorry, I added the disable. >>>>> >>>>>> >>>>>>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts >>>>>> >>>>>> Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not >>>>>> tegra2-seaboard.dts to match the naming in the kernel? >>>>>> >>>>>>> @@ -44,4 +49,9 @@ >>>>>>> usb@c5004000 { >>>>>>> status = "disabled"; >>>>>>> }; >>>>>>> + >>>>>>> + i2c@7000c400 { >>>>>>> + status = "disabled"; >>>>>>> + }; >>>>>>> + >>>>>>> }; >>>>>> >>>>>> That chunk wasn't in the original patch, and doesn't match the kernel's >>>>>> .dts file (and I believe that I2C controller really is in use, so >>>>>> shouldn't be disabled). >>>>> >>>>> It cannot be used - remember the discussion about pinmux? We elected >>>>> to disable I2C1 at present since you didn't like my nvidia,pinmux >>>>> binding for selecting which value to pass to funcmux. The fix is to >>>>> pass 1 instead of 0 for that port, but we have no clean way to specify >>>>> this. >>>>> >>>>> Rather disable it than leave it enabled and not working. >>>> >>>> Rather than having the .dts file not correctly describe the HW, wouldn't >>>> it be better to limit U-Boot's to only initializing the 1 I2C controller >>>> that it knows the valid pinmux setting for? >>> >>> Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not >>> used or connected to anything, so it is a reasonable description of >>> the hardware. The other 3 pinmux settings are valid. >> >> As best I can tell from my schematics, all 4 I2C ports are used on both >> Seaboard and Springbank. > > I don't know about Springbank, but this patch is for Seaboard. > > There is nothing on the bus and I can't see anything on the schematic. > Can you please tell me what it is used for and which pins you are > referring to? GEN2_I2C_SCL/SDA are connected to: J4 (battery or charger) TPM J6 (touchscreen connector) J26 (mini PCIe) J18 (satellite board) I assume you have access to the schematics?
Hi Stephen, On Fri, Feb 3, 2012 at 4:41 PM, Stephen Warren <swarren@nvidia.com> wrote: > On 02/03/2012 05:36 PM, Simon Glass wrote: >> Hi Stephen, >> >> On Fri, Feb 3, 2012 at 4:25 PM, Stephen Warren <swarren@nvidia.com> wrote: >>> On 02/03/2012 05:19 PM, Simon Glass wrote: >>>> Hi Stephen, >>>> >>>> On Fri, Feb 3, 2012 at 4:14 PM, Stephen Warren <swarren@nvidia.com> wrote: >>>>> On 02/03/2012 04:24 PM, Simon Glass wrote: >>>>>> Hi Stephen, >>>>>> >>>>>> On Thu, Jan 19, 2012 at 12:56 PM, Stephen Warren <swarren@nvidia.com> wrote: >>>>>>> On 01/12/2012 12:00 PM, Simon Glass wrote: >>>>>>>> Select the port ordering for I2C on Seaboard. >>>>>>>> >>>>>>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>>>>>> Acked-by: Stephen Warren <swarren@nvidia.com> >>>>>>> >>>>>>> This isn't the patch that I ack'd. >>>>>> >>>>>> Sorry, I added the disable. >>>>>> >>>>>>> >>>>>>>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts >>>>>>> >>>>>>> Unrelated to this patch, but shouldn't that be tegra-seaboard.dts not >>>>>>> tegra2-seaboard.dts to match the naming in the kernel? >>>>>>> >>>>>>>> @@ -44,4 +49,9 @@ >>>>>>>> usb@c5004000 { >>>>>>>> status = "disabled"; >>>>>>>> }; >>>>>>>> + >>>>>>>> + i2c@7000c400 { >>>>>>>> + status = "disabled"; >>>>>>>> + }; >>>>>>>> + >>>>>>>> }; >>>>>>> >>>>>>> That chunk wasn't in the original patch, and doesn't match the kernel's >>>>>>> .dts file (and I believe that I2C controller really is in use, so >>>>>>> shouldn't be disabled). >>>>>> >>>>>> It cannot be used - remember the discussion about pinmux? We elected >>>>>> to disable I2C1 at present since you didn't like my nvidia,pinmux >>>>>> binding for selecting which value to pass to funcmux. The fix is to >>>>>> pass 1 instead of 0 for that port, but we have no clean way to specify >>>>>> this. >>>>>> >>>>>> Rather disable it than leave it enabled and not working. >>>>> >>>>> Rather than having the .dts file not correctly describe the HW, wouldn't >>>>> it be better to limit U-Boot's to only initializing the 1 I2C controller >>>>> that it knows the valid pinmux setting for? >>>> >>>> Well, on Seaboard I2C2 (sorry, not I2C1 as I said in my email) is not >>>> used or connected to anything, so it is a reasonable description of >>>> the hardware. The other 3 pinmux settings are valid. >>> >>> As best I can tell from my schematics, all 4 I2C ports are used on both >>> Seaboard and Springbank. >> >> I don't know about Springbank, but this patch is for Seaboard. >> >> There is nothing on the bus and I can't see anything on the schematic. >> Can you please tell me what it is used for and which pins you are >> referring to? > > GEN2_I2C_SCL/SDA are connected to: > > J4 (battery or charger) > TPM > J6 (touchscreen connector) > J26 (mini PCIe) > J18 (satellite board) > > I assume you have access to the schematics? OK I see - in U-Boot the TPM will be used even if the others aren't. But the point is that I cannot set the correct pinmux value for that port, since you don't want 'nvidia,pinmux = <1>' in the device tree file. So I think I need to disable it for Seaboard. Regards, Simon > > -- > nvpublic
diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts index 7a4ecae..d2cc428 100644 --- a/board/nvidia/dts/tegra2-seaboard.dts +++ b/board/nvidia/dts/tegra2-seaboard.dts @@ -15,6 +15,11 @@ /* This defines the order of our USB ports */ usb0 = "/usb@c5008000"; usb1 = "/usb@c5000000"; + + i2c0 = "/i2c@7000d000"; + i2c1 = "/i2c@7000c000"; + i2c2 = "/i2c@7000c400"; + i2c3 = "/i2c@7000c500"; }; memory { @@ -44,4 +49,9 @@ usb@c5004000 { status = "disabled"; }; + + i2c@7000c400 { + status = "disabled"; + }; + };