| Message ID | 1374570405-8301-2-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/23/2013 04:06 AM, Tomi Valkeinen wrote: > Panda's DVI connector's DDC pins are connected to OMAP's third i2c bus. > With non-DT, the bus number was 3, and that is what is used in the > dss-common.c which contains the platform data for Panda's DVI. > > However, with DT, the bus number is 2. As we now only have DT boot for > Panda, we have to change the bus number to make DVI EDID read > operational. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > arch/arm/mach-omap2/dss-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-omap2/dss-common.c b/arch/arm/mach-omap2/dss-common.c > index 393aeef..043e570 100644 > --- a/arch/arm/mach-omap2/dss-common.c > +++ b/arch/arm/mach-omap2/dss-common.c > @@ -42,7 +42,7 @@ > > /* Using generic display panel */ > static struct tfp410_platform_data omap4_dvi_panel = { > - .i2c_bus_num = 3, > + .i2c_bus_num = 2, > .power_down_gpio = PANDA_DVI_TFP410_POWER_DOWN_GPIO, > }; > > completely dumb question :) : why cant we have dts support for this instead of having to renumber every time? Example: if -EPROBEDEFER is incurred due to some unexpected dependency, we'd have to redo the numbering in the kernel yet again.
On 02/08/13 15:54, Nishanth Menon wrote: > On 07/23/2013 04:06 AM, Tomi Valkeinen wrote: >> Panda's DVI connector's DDC pins are connected to OMAP's third i2c bus. >> With non-DT, the bus number was 3, and that is what is used in the >> dss-common.c which contains the platform data for Panda's DVI. >> >> However, with DT, the bus number is 2. As we now only have DT boot for >> Panda, we have to change the bus number to make DVI EDID read >> operational. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> arch/arm/mach-omap2/dss-common.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/mach-omap2/dss-common.c >> b/arch/arm/mach-omap2/dss-common.c >> index 393aeef..043e570 100644 >> --- a/arch/arm/mach-omap2/dss-common.c >> +++ b/arch/arm/mach-omap2/dss-common.c >> @@ -42,7 +42,7 @@ >> >> /* Using generic display panel */ >> static struct tfp410_platform_data omap4_dvi_panel = { >> - .i2c_bus_num = 3, >> + .i2c_bus_num = 2, >> .power_down_gpio = PANDA_DVI_TFP410_POWER_DOWN_GPIO, >> }; >> >> > completely dumb question :) : > why cant we have dts support for this instead of having to renumber > every time? What do you mean "every time"? Feel free to help me develop the DT support for DSS =). When that's done, we can remove all this code. > Example: if -EPROBEDEFER is incurred due to some unexpected dependency, > we'd have to redo the numbering in the kernel yet again. Hmm, sorry? Do you mean that the i2c bus numbers can change "randomly"? With board files they were numbered 1, 2, 3, 4, but with DT boot they seem to be 0, 1, 2, 3. And as we have the current situation where omap4 boots with DT, but DSS does not have DT support, we add the DSS related devices in a board-file-like-fashion. Tomi
On 08/02/2013 08:00 AM, Tomi Valkeinen wrote: > On 02/08/13 15:54, Nishanth Menon wrote: >> On 07/23/2013 04:06 AM, Tomi Valkeinen wrote: >>> Panda's DVI connector's DDC pins are connected to OMAP's third i2c bus. >>> With non-DT, the bus number was 3, and that is what is used in the >>> dss-common.c which contains the platform data for Panda's DVI. >>> >>> However, with DT, the bus number is 2. As we now only have DT boot for >>> Panda, we have to change the bus number to make DVI EDID read >>> operational. >>> >>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >>> --- >>> arch/arm/mach-omap2/dss-common.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/mach-omap2/dss-common.c >>> b/arch/arm/mach-omap2/dss-common.c >>> index 393aeef..043e570 100644 >>> --- a/arch/arm/mach-omap2/dss-common.c >>> +++ b/arch/arm/mach-omap2/dss-common.c >>> @@ -42,7 +42,7 @@ >>> >>> /* Using generic display panel */ >>> static struct tfp410_platform_data omap4_dvi_panel = { >>> - .i2c_bus_num = 3, >>> + .i2c_bus_num = 2, >>> .power_down_gpio = PANDA_DVI_TFP410_POWER_DOWN_GPIO, >>> }; >>> >>> >> completely dumb question :) : >> why cant we have dts support for this instead of having to renumber >> every time? > > What do you mean "every time"? below > > Feel free to help me develop the DT support for DSS =). When that's > done, we can remove all this code. suggesting here - but it will be nice we have some steps towards this direction - is there anything you'd suggest we do first? > >> Example: if -EPROBEDEFER is incurred due to some unexpected dependency, >> we'd have to redo the numbering in the kernel yet again. > > Hmm, sorry? Do you mean that the i2c bus numbers can change "randomly"? > > With board files they were numbered 1, 2, 3, 4, but with DT boot they > seem to be 0, 1, 2, 3. And as we have the current situation where omap4 > boots with DT, but DSS does not have DT support, we add the DSS related > devices in a board-file-like-fashion. Aah we are then depending on the following i2c-omap.c: adap->nr = pdev->id; r = i2c_add_numbered_adapter(adap); mach-omapx/i2c.c: .. i2c_bus_register.. pdev->id = bus_id platform_device_register(pdev); I dont seem to see any similar numbering enforced in dtsi - if I reorder the i2c entries in SoC dtsi, i'd have an issue as well, no? probe deferral could create problems as well.. not saying it can change random order on a given kernel between boots, but am saying that when new changes come in, deferral mechanism ensures proper ordering dynamically, which kind of messes up with expected device numbers. I had an issue with mmc sometime back where probe deferal on mmc1 made it registered as second device as the dependency was not present on the mmc2. No reason this cannot happen on other busses as well. To solve this on i2c dts, the usual convention is as follows (from omap3-beagle): &i2c1 { clock-frequency = <2600000>; twl: twl@48 { reg = <0x48>; interrupts = <7>; /* SYS_NIRQ cascaded to intc */ interrupt-parent = <&intc>; }; }; this ensures that probe success order is made independent of device expectation of i2c bus numbering or the order in which it was defined in dts. just my 2 cents to help continued functionality in the world of dts..
On 02/08/13 16:30, Nishanth Menon wrote: > On 08/02/2013 08:00 AM, Tomi Valkeinen wrote: >> Feel free to help me develop the DT support for DSS =). When that's >> done, we can remove all this code. > > suggesting here - but it will be nice we have some steps towards this > direction - is there anything you'd suggest we do first? I do have a DSS-DT branch, which works. I've had such a branch for almost a year, although I've been refining it all the time. But I'm still not happy with the DT bindings I have there, and I haven't had any clear ideas how to solve those issues. And, as the DT bindings are an external API, I've been very cautious with it. I'm currently cleaning the branch up, and I hope I can send the series next week. Hopefully I'll receive some ideas for the problematic parts. I'll cc you =). >>> Example: if -EPROBEDEFER is incurred due to some unexpected dependency, >>> we'd have to redo the numbering in the kernel yet again. >> >> Hmm, sorry? Do you mean that the i2c bus numbers can change "randomly"? >> >> With board files they were numbered 1, 2, 3, 4, but with DT boot they >> seem to be 0, 1, 2, 3. And as we have the current situation where omap4 >> boots with DT, but DSS does not have DT support, we add the DSS related >> devices in a board-file-like-fashion. > > Aah we are then depending on the following > i2c-omap.c: > adap->nr = pdev->id; > r = i2c_add_numbered_adapter(adap); > > mach-omapx/i2c.c: > .. i2c_bus_register.. > pdev->id = bus_id > platform_device_register(pdev); > > > I dont seem to see any similar numbering enforced in dtsi - if I reorder > the i2c entries in SoC dtsi, i'd have an issue as well, no? I don't know, but yes, possibly. But you probably won't reorder them, as they are in the HW I2C ID module order. > probe deferral could create problems as well.. not saying it can change > random order on a given kernel between boots, but am saying that when > new changes come in, deferral mechanism ensures proper ordering > dynamically, which kind of messes up with expected device numbers. I had > an issue with mmc sometime back where probe deferal on mmc1 made it > registered as second device as the dependency was not present on the mmc2. Yes, interesting point. If I got you right, you're saying that if a single i2c device gets deferred, but the others not, the ID numbering could change? I don't if it's possible to get only single i2c device deferred. Maybe. Anyway, we need some way to fix this for 3.11. As far as I understand, it's rather unlikely that the bus number would change, so I think this fix is the best I can do. > No reason this cannot happen on other busses as well. To solve this on > i2c dts, the usual convention is as follows (from omap3-beagle): > &i2c1 { > clock-frequency = <2600000>; > > twl: twl@48 { > reg = <0x48>; > interrupts = <7>; /* SYS_NIRQ cascaded to intc */ > interrupt-parent = <&intc>; > }; > }; > > this ensures that probe success order is made independent of device > expectation of i2c bus numbering or the order in which it was defined in > dts. With dts, this is not a problem, it's handle with phandles. Although not as you show above, because we don't really have an i2c device on the board. The i2c in question goes through the DVI connector to the monitor. So the monitor is the i2c device. Thus we just get a handle to the i2c bus going to the monitor, and send messages to it directly, without having a linux i2c device. Tomi
On 08/02/2013 09:22 AM, Tomi Valkeinen wrote: > On 02/08/13 16:30, Nishanth Menon wrote: >> On 08/02/2013 08:00 AM, Tomi Valkeinen wrote: > >>> Feel free to help me develop the DT support for DSS =). When that's >>> done, we can remove all this code. >> >> suggesting here - but it will be nice we have some steps towards this >> direction - is there anything you'd suggest we do first? > > I do have a DSS-DT branch, which works. I've had such a branch for > almost a year, although I've been refining it all the time. But I'm > still not happy with the DT bindings I have there, and I haven't had any > clear ideas how to solve those issues. And, as the DT bindings are an > external API, I've been very cautious with it. > > I'm currently cleaning the branch up, and I hope I can send the series > next week. Hopefully I'll receive some ideas for the problematic parts. > I'll cc you =). Awesome, given that there is work in progress to do this properly and the current solution is meant for 3.11 transition, I dont have any objections. > >>>> Example: if -EPROBEDEFER is incurred due to some unexpected dependency, >>>> we'd have to redo the numbering in the kernel yet again. >>> >>> Hmm, sorry? Do you mean that the i2c bus numbers can change "randomly"? >>> >>> With board files they were numbered 1, 2, 3, 4, but with DT boot they >>> seem to be 0, 1, 2, 3. And as we have the current situation where omap4 >>> boots with DT, but DSS does not have DT support, we add the DSS related >>> devices in a board-file-like-fashion. >> >> Aah we are then depending on the following >> i2c-omap.c: >> adap->nr = pdev->id; >> r = i2c_add_numbered_adapter(adap); >> >> mach-omapx/i2c.c: >> .. i2c_bus_register.. >> pdev->id = bus_id >> platform_device_register(pdev); >> >> >> I dont seem to see any similar numbering enforced in dtsi - if I reorder >> the i2c entries in SoC dtsi, i'd have an issue as well, no? > > I don't know, but yes, possibly. But you probably won't reorder them, as > they are in the HW I2C ID module order. we dont want to depend on ordering of anything usually.. anyways, besides the point, I guess. > >> probe deferral could create problems as well.. not saying it can change >> random order on a given kernel between boots, but am saying that when >> new changes come in, deferral mechanism ensures proper ordering >> dynamically, which kind of messes up with expected device numbers. I had >> an issue with mmc sometime back where probe deferal on mmc1 made it >> registered as second device as the dependency was not present on the mmc2. > > Yes, interesting point. If I got you right, you're saying that if a > single i2c device gets deferred, but the others not, the ID numbering > could change? > > I don't if it's possible to get only single i2c device deferred. Maybe. > > Anyway, we need some way to fix this for 3.11. As far as I understand, > it's rather unlikely that the bus number would change, so I think this > fix is the best I can do. > >> No reason this cannot happen on other busses as well. To solve this on >> i2c dts, the usual convention is as follows (from omap3-beagle): >> &i2c1 { >> clock-frequency = <2600000>; >> >> twl: twl@48 { >> reg = <0x48>; >> interrupts = <7>; /* SYS_NIRQ cascaded to intc */ >> interrupt-parent = <&intc>; >> }; >> }; >> >> this ensures that probe success order is made independent of device >> expectation of i2c bus numbering or the order in which it was defined in >> dts. > > With dts, this is not a problem, it's handle with phandles. Although not > as you show above, because we don't really have an i2c device on the board. > > The i2c in question goes through the DVI connector to the monitor. So > the monitor is the i2c device. Thus we just get a handle to the i2c bus > going to the monitor, and send messages to it directly, without having a > linux i2c device. I think we should take this as part of your series next week :). I could argue that the device on remote display which communicates over DCC is an real i2c device and communicates using real i2c protocol? when we read it, how the gates to EDID information is opened up is upto the driver.
diff --git a/arch/arm/mach-omap2/dss-common.c b/arch/arm/mach-omap2/dss-common.c index 393aeef..043e570 100644 --- a/arch/arm/mach-omap2/dss-common.c +++ b/arch/arm/mach-omap2/dss-common.c @@ -42,7 +42,7 @@ /* Using generic display panel */ static struct tfp410_platform_data omap4_dvi_panel = { - .i2c_bus_num = 3, + .i2c_bus_num = 2, .power_down_gpio = PANDA_DVI_TFP410_POWER_DOWN_GPIO, };
Panda's DVI connector's DDC pins are connected to OMAP's third i2c bus. With non-DT, the bus number was 3, and that is what is used in the dss-common.c which contains the platform data for Panda's DVI. However, with DT, the bus number is 2. As we now only have DT boot for Panda, we have to change the bus number to make DVI EDID read operational. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- arch/arm/mach-omap2/dss-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)