* Re: [PATCH 00/33] OMAPDSS: platform_enable/disable callback removal from panel drivers
From: Archit Taneja @ 2013-02-14 7:29 UTC (permalink / raw)
To: Tony Lindgren; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <20130213164647.GF7144@atomide.com>
Hi,
On Wednesday 13 February 2013 10:16 PM, Tony Lindgren wrote:
> * Archit Taneja <archit@ti.com> [130213 06:26]:
>> init functions in omap board files request panel specific gpios, and provide
>> functions which omapdss panel drivers call to enable or disable them.
>>
>> Instead of the board files requesting these gpios, they should just pass the
>> platform specific data(like the gpio numbers), the panel should retrieve the
>> platform data and request the gpios. Doing this prevents the need of the panel
>> driver calling platform functions in board files.
>>
>> Panel drivers have their own platform data struct, and the board files populate
>> these structs and pass the pointer to the 'data' field of omap_dss_device. This
>> work will make it easier for the panel drivers be more adaptable to the
>> DT model.
>>
>> There is also removal of passing panel reset_gpio numbers through
>> omap_dss_device struct directly, reset gpios are passed through platform data
>> only.
>
> To avoid merge conflicts and dependencies between drivers and core
> Soc code, please break thes kind of patches into following parts:
>
> 1. Any platform_data header changes needed so both I and Tomi
> can pull it in as needed.
>
> 2. Changes to DSS drivers. Please keep stubs around for the
> board specific callback functions so omap2plus_defconfig
> won't break with just #1 merged into arm soc tree.
The build won't break, and the kernel will boot up properly, but the
panels won't work till the time #3 is also merged,
>
> 3. All the arch/arm/*omap* changes based on #1 above to
> drop obsolete callback functions and add new pdata if still
> needed. This needs to build and boot on #1 so I can merge
> this in via arm soc tree.
>
> 4. Any .dts changes needed.
We don't have any .dts changes for DSS as of now.
I'll split the patches accordingly.
Thanks,
Archit
^ permalink raw reply
* Re: [PATCH 16/33] OMAPDSS: acx565akm panel: handle gpios in panel driver
From: Archit Taneja @ 2013-02-14 7:20 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Aaro Koskinen, linux-omap, linux-fbdev
In-Reply-To: <511C8B16.5060605@ti.com>
On Thursday 14 February 2013 12:28 PM, Tomi Valkeinen wrote:
> On 2013-02-14 08:51, Archit Taneja wrote:
>> On Wednesday 13 February 2013 10:59 PM, Aaro Koskinen wrote:
>>> Hi,
>>>
>>> On Wed, Feb 13, 2013 at 07:52:08PM +0530, Archit Taneja wrote:
>>>> +static struct panel_acx565akm_data *get_panel_data(struct
>>>> omap_dss_device *dssdev)
>>>> +{
>>>> + return (struct panel_acx565akm_data *) dssdev->data;
>>>> +}
>>>> +
>>>> static int acx_panel_probe(struct omap_dss_device *dssdev)
>>>> {
>>>> int r;
>>>> struct acx565akm_device *md = &acx_dev;
>>>> + struct panel_acx565akm_data *panel_data = get_panel_data(dssdev);
>>>
>>> Why the get_panel_data function is needed, isn't the cast unnecessary?
>>
>> the 'data' member of omap_dss_device has the type 'void *', we need to
>> cast it to access the panel_acx565akm_data struct pointer.
>
> You don't need an explicit cast to assign a void pointer to a pointer to
> something else (or vice versa, I think).
>
> I remember us having similar constructs in some other panel drivers
> also. I think they are unnecessary also.
Ah okay, I'll take care of it.
Archit
^ permalink raw reply
* Re: [PATCH 05/33] arm: omap: board-cm-t35: use generic dpi panel's gpio handling
From: Tomi Valkeinen @ 2013-02-14 7:09 UTC (permalink / raw)
To: Igor Grinberg; +Cc: Archit Taneja, linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <511C8A83.5070407@compulab.co.il>
[-- Attachment #1: Type: text/plain, Size: 1653 bytes --]
On 2013-02-14 08:56, Igor Grinberg wrote:
> On 02/13/13 17:59, Tomi Valkeinen wrote:
>> Okay, so I just realized there's an spi backlight driver used here, and
>> that backlight driver is actually handling the SPI transactions with the
>> panel, instead of the panel driver. So this looks quite messed up.
>
> Yep, it always was.
> The whole DSS specific panel handling inside the
> drivers/video/omap2/displays is a mess.
Well, that's not mess itself, it's just omap specific panel framework.
But dividing single device handling into two separate places is a mess.
> Those panels can be (and are) used not only with OMAP based boards.
True, but as there's no generic panel framework, that's the best we can
do. But see CDF (common display framework) discussions if you're
interested in what's hopefully coming soon.
>> For a quick solution, can we just set the LCD_EN at boot time (with the
>> msleep), and not touch it after that?
>
> That would be sensible for now, so this series can be merged.
> As a more appropriate (and long term) solution,
> I plan on moving the panel reset pin handling to the spi backlight
> driver itself.
Well, if you must. But I suggest moving the whole panel handling into a
(omap specific) panel driver, as it's done for other panels. That way
you'll have a proper panel driver for it, for omap, and when CDF comes,
you'll get a platform independent panel driver for it.
Of course, if you have multiple platforms already using that backlight
driver, the omap specific approach may not be enticing. So perhaps it's
easier to just do the quick fix and wait for CDF.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 16/33] OMAPDSS: acx565akm panel: handle gpios in panel driver
From: Tomi Valkeinen @ 2013-02-14 6:58 UTC (permalink / raw)
To: Archit Taneja; +Cc: Aaro Koskinen, linux-omap, linux-fbdev
In-Reply-To: <511C896C.10007@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]
On 2013-02-14 08:51, Archit Taneja wrote:
> On Wednesday 13 February 2013 10:59 PM, Aaro Koskinen wrote:
>> Hi,
>>
>> On Wed, Feb 13, 2013 at 07:52:08PM +0530, Archit Taneja wrote:
>>> +static struct panel_acx565akm_data *get_panel_data(struct
>>> omap_dss_device *dssdev)
>>> +{
>>> + return (struct panel_acx565akm_data *) dssdev->data;
>>> +}
>>> +
>>> static int acx_panel_probe(struct omap_dss_device *dssdev)
>>> {
>>> int r;
>>> struct acx565akm_device *md = &acx_dev;
>>> + struct panel_acx565akm_data *panel_data = get_panel_data(dssdev);
>>
>> Why the get_panel_data function is needed, isn't the cast unnecessary?
>
> the 'data' member of omap_dss_device has the type 'void *', we need to
> cast it to access the panel_acx565akm_data struct pointer.
You don't need an explicit cast to assign a void pointer to a pointer to
something else (or vice versa, I think).
I remember us having similar constructs in some other panel drivers
also. I think they are unnecessary also.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 05/33] arm: omap: board-cm-t35: use generic dpi panel's gpio handling
From: Igor Grinberg @ 2013-02-14 6:56 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <511BB87E.60003@ti.com>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/13/13 17:59, Tomi Valkeinen wrote:
> On 2013-02-13 17:28, Tomi Valkeinen wrote:
>> On 2013-02-13 17:16, Igor Grinberg wrote:
>>> Hi Archit,
>>>
>>> On 02/13/13 16:21, Archit Taneja wrote:
>>>> The cm-t35 board file currently requests gpios required to configure the tdo35s
>>>> panel, and provides platform_enable/disable callbacks to configure them.
>>>>
>>>> These tasks have been moved to the generic dpi panel driver itself and shouldn't
>>>> be done in the board files.
>>>>
>>>> Remove the gpio requests and the platform callbacks from the board file.
>>>> Add the gpio information to generic dpi panel's platform data so that it's
>>>> passed to the panel driver.
>>>>
>>>> Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
>>>> enabled after a 50 millisecond delay. This code has been removed and is not
>>>> taken care of in the generic panel driver. The impact of this needs to be
>>>> tested. The panel's gpios are also not exported any more. Hence, they can't be
>>>> accessed via sysfs interface.
>>>
>>> Indeed, there is an impact - the LCD no longer works.
>>> The reason for the LCD_EN gpio being pushed high after the 50ms delay,
>>> is to get the LCD out of reset, so the SPI transaction will succeed
>>> and initialize the LCD.
>>> Now, when you remove the gpio handling for the LCD_EN pin,
>>> the LCD no longer works.
>>
>> So between what is the sleep done? It's not clear from the code. LCD_EN
>> needs to be 0 for 50ms, or...?
>>
>> If the panel requires specific reset handling, does it work right even
>> currently when the panel is turned off and later turned on? The msleep
>> is only used at boot time.
>
> Okay, so I just realized there's an spi backlight driver used here, and
> that backlight driver is actually handling the SPI transactions with the
> panel, instead of the panel driver. So this looks quite messed up.
Yep, it always was.
The whole DSS specific panel handling inside the
drivers/video/omap2/displays is a mess.
Those panels can be (and are) used not only with OMAP based boards.
>
> For a quick solution, can we just set the LCD_EN at boot time (with the
> msleep), and not touch it after that?
That would be sensible for now, so this series can be merged.
As a more appropriate (and long term) solution,
I plan on moving the panel reset pin handling to the spi backlight
driver itself.
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRHIqCAAoJEBDE8YO64Efamw0P/R2wt0tpCI1ecrnutVGMX4bF
Pyjk2B65uDWqoqZ/cpJUqnvmupXl5UdrA7eqKjTBh1A+g81UVFcNDMuJsbPIIiYI
1pimZieAq0T6Vag00PKImKlkhJYfC7JVBbESij/NONlzYtPkbZ91Y+Ik4DZXnyZf
1TS4GbHQ25tjl73PkwlzLUcJIDIogsimSrkM+aWXPE8LmvrBEQs0LhAObPsAFtgL
1An3hvA2Tkhh9QgerWQd9YiqX994tv1PGRLBEXTbjh1yihzKSNleuvw3NdM+wf9i
9Y5l9IV6L2dtYBMLCpzkiGQBDdOzoq+fObRnSgK6Kr1mfXot+MAlLrk9gCeWcq1b
c2oU/imKWB4sZys21pTnjIxAIzzRDoGW40qXuibTW4DoAYaVHuEBPtphjMVCBCcQ
sJaIVXpsChQ3vvtHOgllnInMjCnlXJ3Piqr4y5glTPxu9mZHdPr6VDpWdqRmvyr9
V7fRQztwXB3Td+SZVDD1yBqoXKlKCX4QPlLAqH3FI9s1WhDHcJePcoDJY0/QyXB1
IeQRlEwBBEZAYy/kr9/pwbZzXeh5V5dK6wAq8aT+thS22zl3nJbKjW//vN06+ib+
WAnHRSZ8iCbUX2cRVF1k+DCQOTi8QCbI6WTcLsgenLeSrbEuzilfgsrsvd6LHfjD
oTODiiD9QInP2sBfknUp
=tWsB
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH 16/33] OMAPDSS: acx565akm panel: handle gpios in panel driver
From: Archit Taneja @ 2013-02-14 6:52 UTC (permalink / raw)
To: Aaro Koskinen; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <20130213172913.GD21750@blackmetal.musicnaut.iki.fi>
On Wednesday 13 February 2013 10:59 PM, Aaro Koskinen wrote:
> Hi,
>
> On Wed, Feb 13, 2013 at 07:52:08PM +0530, Archit Taneja wrote:
>> +static struct panel_acx565akm_data *get_panel_data(struct omap_dss_device *dssdev)
>> +{
>> + return (struct panel_acx565akm_data *) dssdev->data;
>> +}
>> +
>> static int acx_panel_probe(struct omap_dss_device *dssdev)
>> {
>> int r;
>> struct acx565akm_device *md = &acx_dev;
>> + struct panel_acx565akm_data *panel_data = get_panel_data(dssdev);
>
> Why the get_panel_data function is needed, isn't the cast unnecessary?
the 'data' member of omap_dss_device has the type 'void *', we need to
cast it to access the panel_acx565akm_data struct pointer.
Archit
^ permalink raw reply
* Re: [PATCH 27/33] OMAPDSS: n8x0 panel: handle gpio data in panel driver
From: Archit Taneja @ 2013-02-14 6:46 UTC (permalink / raw)
To: Aaro Koskinen; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <20130213173527.GE21750@blackmetal.musicnaut.iki.fi>
Hi,
On Wednesday 13 February 2013 11:05 PM, Aaro Koskinen wrote:
> Hi,
>
> On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
>> @@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
>> dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
>> dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
>>
>> + if (gpio_is_valid(bdata->panel_reset)) {
>> + r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
>> + GPIOF_OUT_INIT_LOW, "PANEL RESET");
>> + if (r)
>> + return r;
>> + }
>> +
>> + if (gpio_is_valid(bdata->ctrl_pwrdown)) {
>> + r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
>> + GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
>> + if (r)
>> + return r;
>> + }
>> +
>
> In the error case, the other GPIO is not freed. Also maybe you should
> free them on module removal, because now the module owns the GPIOs.
Wouldn't the usage of devm_* functions take care of this? If the device
isn't registered successfully, then all allocations/requests done using
devm_* functions will be free'd automatically.
Archit
^ permalink raw reply
* Re: [PATCH v2 0/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Ruslan Bilovol @ 2013-02-14 0:07 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: andi, FlorianSchandinat, linux-fbdev, linux-kernel, linux-omap
In-Reply-To: <511B4C60.50404@ti.com>
Hi Tomi,
On Wed, Feb 13, 2013 at 10:18 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> On 2013-02-08 17:43, Ruslan Bilovol wrote:
>> Hi,
>>
>> This patch adds support for TC358765 DSI-to-LVDS transmitter
>> from Toshiba, that is used in OMAP4 Blaze Tablet development
>> platform. It was originally developed a long time ago and
>> was used internally survived few kernel migrations.
>> Different people worked in this driver during last two
>> years changing its sources to what each new version of
>> kernel expects.
>>
>> Current version is ported from internal kernel v3.4 to 3.8.0-rc6
>> Tested basic functioning under busybox.
>
> Thanks for updating the driver to a recent kernel.
>
> I didn't review the patch line by line, but with a brief look it looks
> fine. However, I'm not quite sure what to do with this patch.
>
> The problem is that the driver is a combined driver for the TC358765
> chip and the panel on Blaze Tablet, and we are very much trying to fix
> that design issue so that the drivers for the chip and panel could be
> separate, as they should.
>
> So I fear that if I now accept the patch, it'll increase my burden of
> managing the display drivers during this design rework. Furthermore, to
> my knowledge the Blaze Tablet is quite a rare board, and it's not even
> supported in the mainline kernel, so this driver wouldn't be usable by
> itself.
This patch is exactly a part of mainlining of the BlazeTablet board support :)
The goal is to have as much as possible support of this board in the
'vanilla' kernel.
The BlazeTablet mainlining is already ongoing (
https://patchwork.kernel.org/patch/2118281/ )
and I hope we will have some support of this board in near future.
The BlazeTablet's LCD panel support is very important thing for us to
have it in mainline because
without it the BlazeTablet becomes a 'black brick' and it is painful
for us to port this
driver against each new version of kernel.
>
> So if I'm right about the blaze tablet status, I'm inclined to say that
> I think this should be still kept out of tree.
Considering my information above, (and your information that design
rework will be started later), can we pick this driver for now in its
current state?
Regards,
Ruslan
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH RESEND v2 2/2] drivers/video: fsl-diu-fb: fix bugs in interrupt handling
From: Anatolij Gustschin @ 2013-02-13 20:13 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360308959-3096-2-git-send-email-agust@denx.de>
On Wed, 13 Feb 2013 11:55:24 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 13 Feb 2013 10:21:52 +0100
> Anatolij Gustschin <agust@denx.de> wrote:
>
> > On Tue, 12 Feb 2013 17:01:55 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > > On Tue, 12 Feb 2013 16:54:58 -0800
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > >
> > > > Thanks, I queued both these with a plan to merge into 3.9-rc1.
> > >
> > > No I didn't. The patches have already found their way into linux-next
> > > via some other tree.
> >
> > Yes, via MPC5XXX tree. Since I didn't receive response from fbdev
> > maintainer I applied both patches in my powerpc MPC5XXX next branch
> > so that these can be exposed in linux-next at least. MPC5121 uses the
> > fsl-diu-fb driver, so these patches could go via MPC5XXX tree, but I
> > can't push them via my tree without an ACK from fbdev maintainer.
>
> The patches look OK to me - I suggest you go ahead and merge them up
> for 3.9-rc1.
Okay, will do.
> > > Without any -stable tags, either. You don't think we should fix the
> > > "24 and 16 bpp" thing in 3.8.x and earlier?
> >
> > I didn't add any -stable tags since my hope was that the patches
> > will go into v3.8 via fbdev tree. It would be good to fix the bpp
> > issue in 3.8.x. But the issue is not critical for earlier maintained
> > stable versions I think.
>
> Please remember to add the cc:stable to the changelogs if you want
> these in 3.8.x and earlier.
Okay, thanks!
Anatolij
^ permalink raw reply
* Re: [PATCH RESEND v2 2/2] drivers/video: fsl-diu-fb: fix bugs in interrupt handling
From: Andrew Morton @ 2013-02-13 19:55 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1360308959-3096-2-git-send-email-agust@denx.de>
On Wed, 13 Feb 2013 10:21:52 +0100
Anatolij Gustschin <agust@denx.de> wrote:
> On Tue, 12 Feb 2013 17:01:55 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > On Tue, 12 Feb 2013 16:54:58 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > > Thanks, I queued both these with a plan to merge into 3.9-rc1.
> >
> > No I didn't. The patches have already found their way into linux-next
> > via some other tree.
>
> Yes, via MPC5XXX tree. Since I didn't receive response from fbdev
> maintainer I applied both patches in my powerpc MPC5XXX next branch
> so that these can be exposed in linux-next at least. MPC5121 uses the
> fsl-diu-fb driver, so these patches could go via MPC5XXX tree, but I
> can't push them via my tree without an ACK from fbdev maintainer.
The patches look OK to me - I suggest you go ahead and merge them up
for 3.9-rc1.
> > Without any -stable tags, either. You don't think we should fix the
> > "24 and 16 bpp" thing in 3.8.x and earlier?
>
> I didn't add any -stable tags since my hope was that the patches
> will go into v3.8 via fbdev tree. It would be good to fix the bpp
> issue in 3.8.x. But the issue is not critical for earlier maintained
> stable versions I think.
Please remember to add the cc:stable to the changelogs if you want
these in 3.8.x and earlier.
^ permalink raw reply
* Re: [PATCH v2 1/1] OMAP4: DSS: Add panel for Blaze Tablet boards
From: Ruslan Bilovol @ 2013-02-13 19:23 UTC (permalink / raw)
To: Andi Shyti
Cc: tomi.valkeinen, FlorianSchandinat, linux-fbdev, linux-kernel,
linux-omap
In-Reply-To: <20130210235125.GA8098@jack.whiskey>
Hi Andi,
Thanks for your review,
On Mon, Feb 11, 2013 at 1:51 AM, Andi Shyti <andi@etezian.org> wrote:
> Hi Ruslan,
>
>> TC358765 is DSI-to-LVDS transmitter from Toshiba, used in
>> OMAP44XX Blaze Tablet and Blaze Tablet2 boards.
>
> I think it's fine, just some nitpicks and checkpatch warnings
>
>> +struct {
>> + struct device *dev;
>> + struct dentry *dir;
>> +} tc358765_debug;
>
> Should this be static?
Agree.
>
>> +struct tc358765_reg {
>> + const char *name;
>> + u16 reg;
>> + u8 perm:2;
>> +} tc358765_regs[] = {
>
> Should this be static as well?
Agree.
>
>> + { "D1S_ZERO", D1S_ZERO, A_RW },
>> + { "D2S_ZERO", D2S_ZERO, A_RW },
>> + { "D3S_ZERO", D3S_ZERO, A_RW },
>> + { "PPI_CLRFLG", PPI_CLRFLG, A_RW },
>> + { "PPI_CLRSIPO", PPI_CLRSIPO, A_RW },
>> + { "PPI_HSTimeout", PPI_HSTimeout, A_RW },
>
> WARNING: Avoid CamelCase: <PPI_HSTimeout>
> #136: FILE: video/omap2/displays/panel-tc358765.c:136:
> + { "PPI_HSTimeout", PPI_HSTimeout, A_RW },
>
>> + { "PPI_HSTimeoutEnable", PPI_HSTimeoutEnable, A_RW },
>
> WARNING: Avoid CamelCase: <PPI_HSTimeoutEnable>
> #137: FILE: video/omap2/displays/panel-tc358765.c:137:
> + { "PPI_HSTimeoutEnable", PPI_HSTimeoutEnable, A_RW },
Hmm... I though these registers had such naming in the documentation,
however, after looking into it I found the names are in upper case there
so this will be fixed.
>
>
>> +static int tc358765_read_block(u16 reg, u8 *data, int len)
>> +{
>> + unsigned char wb[2];
>> + struct i2c_msg msg[2];
>> + int r;
>> + mutex_lock(&tc358765_i2c->xfer_lock);
>> + wb[0] = (reg & 0xff00) >> 8;
>> + wb[1] = reg & 0xff;
>> + msg[0].addr = tc358765_i2c->client->addr;
>> + msg[0].len = 2;
>> + msg[0].flags = 0;
>> + msg[0].buf = wb;
>> + msg[1].addr = tc358765_i2c->client->addr;
>> + msg[1].flags = I2C_M_RD;
>> + msg[1].len = len;
>> + msg[1].buf = data;
>> +
>> + r = i2c_transfer(tc358765_i2c->client->adapter, msg, ARRAY_SIZE(msg));
>> + mutex_unlock(&tc358765_i2c->xfer_lock);
>> +
>> + if (r = ARRAY_SIZE(msg))
>> + return len;
>> +
>> + return r;
>
> If you like, here you could do
>
> return (r = ARRAY_SIZE(msg)) ? len : r;
>
> Usually I like more this notation because keeps the code more
> compact and immediate to read, but you don't have to
Yes, makes sense for "beautification" :)
Will change
>
>> + mutex_lock(&tc358765_i2c->xfer_lock);
>> + ret = i2c_transfer(tc358765_i2c->client->adapter, &msg, 1);
>> + mutex_unlock(&tc358765_i2c->xfer_lock);
>> +
>> + if (ret != 1)
>> + return ret;
>> + return 0;
>
> Also here you could do
>
> return (ret != 1) ? ret : 0;
>
> But this is more taste :)
>
>> +static ssize_t tc358765_seq_write(struct file *filp, const char __user *ubuf,
>> + size_t size, loff_t *ppos)
>> +{
>> + struct device *dev = tc358765_debug.dev;
>> + unsigned i, reg_count;
>> + u32 value = 0;
>> + int error = 0;
>> + /* kids, don't use register names that long */
>
> I won't, promised, but please, drop this comment :)
>
>> + char name[30];
>> + char buf[50];
>> +
>> + if (size >= sizeof(buf))
>> + size = sizeof(buf);
>
> what's the point of this?
This is a way to limit copied from userspace data by available buffer size,
widely used in current kernel sources. Are you implying there is some
better (more graceful) way?
>
>> +static void tc358765_uninitialize_debugfs(void)
>> +{
>> + if (tc358765_debug.dir)
>> + debugfs_remove_recursive(tc358765_debug.dir);
>
> WARNING: debugfs_remove_recursive(NULL) is safe this check is probably not required
> #435: FILE: video/omap2/displays/panel-tc358765.c:435:
> + if (tc358765_debug.dir)
> + debugfs_remove_recursive(tc358765_debug.dir);
Will fix it..
>
>
>> +static struct tc358765_board_data *get_board_data(struct omap_dss_device
>> + *dssdev)
>> +{
>> + return (struct tc358765_board_data *)dssdev->data;
>
> You shouldn't need for this cast, does it complain?
yes, we don't need this :)
>
>> +}
>> +
>> +static void tc358765_get_timings(struct omap_dss_device *dssdev,
>> + struct omap_video_timings *timings)
>> +{
>> + *timings = dssdev->panel.timings;
>> +}
>> +
>> +static void tc358765_set_timings(struct omap_dss_device *dssdev,
>> + struct omap_video_timings *timings)
>> +{
>> + dev_info(&dssdev->dev, "set_timings() not implemented\n");
>
> ... then drop the function :)
I need to check if this will have any side effects in places where
this is used. I mean next:
| if (blabla->set_timings)
| blabla->set_timings();
| else
| do_another_thing();
But it seems this may be safely removed
>
>> + if ((pins[2] & 1) || (pins[3] & 1)) {
>> + lanes |= (1 << 1);
>> + ret |= tc358765_write_register(dssdev, PPI_D0S_CLRSIPOCOUNT,
>> + board_data->clrsipo);
>> + }
>> + if ((pins[4] & 1) || (pins[5] & 1)) {
>> + lanes |= (1 << 2);
>> + ret |= tc358765_write_register(dssdev, PPI_D1S_CLRSIPOCOUNT,
>> + board_data->clrsipo);
>> + }
>> + if ((pins[6] & 1) || (pins[7] & 1)) {
>> + lanes |= (1 << 3);
>> + ret |= tc358765_write_register(dssdev, PPI_D2S_CLRSIPOCOUNT,
>> + board_data->clrsipo);
>> + }
>> + if ((pins[8] & 1) || (pins[9] & 1)) {
>> + lanes |= (1 << 4);
>> + ret |= tc358765_write_register(dssdev, PPI_D3S_CLRSIPOCOUNT,
>> + board_data->clrsipo);
>> + }
>
> Can't this be done in one single multiwrighting command since
> this registers are consecutive?
>
> You build once the array to write and you send it at once.
In this particular case I disagree. Yes, it will be a little bit
faster, however:
1) we write this for panel initialization only (so no impact in other cases)
2) multiwriting of array will make code reading more difficult
So I would like to leave it as-is
Same is for next your similar comment.
>
> Moreover it would be nice to have a name for all those nombers
Okay, will replace magic numbers by defined values
>
>> + ret |= tc358765_write_register(dssdev, HTIM1,
>> + (tc358765_timings.hbp << 16) | tc358765_timings.hsw);
>> + ret |= tc358765_write_register(dssdev, HTIM2,
>> + ((tc358765_timings.hfp << 16) | tc358765_timings.x_res));
>> + ret |= tc358765_write_register(dssdev, VTIM1,
>> + ((tc358765_timings.vbp << 16) | tc358765_timings.vsw));
>> + ret |= tc358765_write_register(dssdev, VTIM2,
>> + ((tc358765_timings.vfp << 16) | tc358765_timings.y_res));
>
> also this and all the other cases I haven't checked
>
>> +static int tc358765_enable(struct omap_dss_device *dssdev)
>> +{
>> + struct tc358765_data *d2d = dev_get_drvdata(&dssdev->dev);
>> + int r = 0;
>
> this initialisation is useless
Agree
>
>> + if (r) {
>> + dev_dbg(&dssdev->dev, "enable failed\n");
>
> Here you could choose a different print level, I would have used
> dev_err instead.
Agree
>
>> +static int tc358765_i2c_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + tc358765_i2c = devm_kzalloc(&client->dev, sizeof(*tc358765_i2c), GFP_KERNEL);
>
> WARNING: line over 80 characters
> #927: FILE: video/omap2/displays/panel-tc358765.c:927:
> + tc358765_i2c = devm_kzalloc(&client->dev, sizeof(*tc358765_i2c), GFP_KERNEL);
Agree :)
>
>
>> + /* store i2c_client pointer on private data structure */
>> + tc358765_i2c->client = client;
>> +
>> + /* store private data structure pointer on i2c_client structure */
>> + i2c_set_clientdata(client, tc358765_i2c);
>> +
>> + /* init mutex */
>> + mutex_init(&tc358765_i2c->xfer_lock);
>> + dev_err(&client->dev, "D2L i2c initialized\n");
>
> while here dev_dbg (or dev_info) are better.
Agree
>
>> +static int __init tc358765_init(void)
>> +{
>> + int r;
>> + tc358765_i2c = NULL;
>> + r = i2c_add_driver(&tc358765_i2c_driver);
>> + if (r < 0) {
>> + printk(KERN_WARNING "d2l i2c driver registration failed\n");
>
> WARNING: Prefer netdev_warn(netdev, ... then dev_warn(dev, ... then pr_warn(... to printk(KERN_WARNING ...
> #981: FILE: video/omap2/displays/panel-tc358765.c:981:
> + printk(KERN_WARNING "d2l i2c driver registration failed\n");
Agree
--
Best regards,
Ruslan
>
>
> Andi
^ permalink raw reply
* Re: [PATCH 27/33] OMAPDSS: n8x0 panel: handle gpio data in panel driver
From: Aaro Koskinen @ 2013-02-13 17:35 UTC (permalink / raw)
To: Archit Taneja; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <1360765345-19312-28-git-send-email-archit@ti.com>
Hi,
On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
> @@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
> dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
> dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
>
> + if (gpio_is_valid(bdata->panel_reset)) {
> + r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
> + GPIOF_OUT_INIT_LOW, "PANEL RESET");
> + if (r)
> + return r;
> + }
> +
> + if (gpio_is_valid(bdata->ctrl_pwrdown)) {
> + r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
> + GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
> + if (r)
> + return r;
> + }
> +
In the error case, the other GPIO is not freed. Also maybe you should
free them on module removal, because now the module owns the GPIOs.
A.
^ permalink raw reply
* Re: [PATCH 16/33] OMAPDSS: acx565akm panel: handle gpios in panel driver
From: Aaro Koskinen @ 2013-02-13 17:29 UTC (permalink / raw)
To: Archit Taneja; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <1360765345-19312-17-git-send-email-archit@ti.com>
Hi,
On Wed, Feb 13, 2013 at 07:52:08PM +0530, Archit Taneja wrote:
> +static struct panel_acx565akm_data *get_panel_data(struct omap_dss_device *dssdev)
> +{
> + return (struct panel_acx565akm_data *) dssdev->data;
> +}
> +
> static int acx_panel_probe(struct omap_dss_device *dssdev)
> {
> int r;
> struct acx565akm_device *md = &acx_dev;
> + struct panel_acx565akm_data *panel_data = get_panel_data(dssdev);
Why the get_panel_data function is needed, isn't the cast unnecessary?
A.
^ permalink raw reply
* Re: [PATCH 00/33] OMAPDSS: platform_enable/disable callback removal from panel drivers
From: Tony Lindgren @ 2013-02-13 16:46 UTC (permalink / raw)
To: Archit Taneja; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
* Archit Taneja <archit@ti.com> [130213 06:26]:
> init functions in omap board files request panel specific gpios, and provide
> functions which omapdss panel drivers call to enable or disable them.
>
> Instead of the board files requesting these gpios, they should just pass the
> platform specific data(like the gpio numbers), the panel should retrieve the
> platform data and request the gpios. Doing this prevents the need of the panel
> driver calling platform functions in board files.
>
> Panel drivers have their own platform data struct, and the board files populate
> these structs and pass the pointer to the 'data' field of omap_dss_device. This
> work will make it easier for the panel drivers be more adaptable to the
> DT model.
>
> There is also removal of passing panel reset_gpio numbers through
> omap_dss_device struct directly, reset gpios are passed through platform data
> only.
To avoid merge conflicts and dependencies between drivers and core
Soc code, please break thes kind of patches into following parts:
1. Any platform_data header changes needed so both I and Tomi
can pull it in as needed.
2. Changes to DSS drivers. Please keep stubs around for the
board specific callback functions so omap2plus_defconfig
won't break with just #1 merged into arm soc tree.
3. All the arch/arm/*omap* changes based on #1 above to
drop obsolete callback functions and add new pdata if still
needed. This needs to build and boot on #1 so I can merge
this in via arm soc tree.
4. Any .dts changes needed.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 05/33] arm: omap: board-cm-t35: use generic dpi panel's gpio handling
From: Tomi Valkeinen @ 2013-02-13 15:59 UTC (permalink / raw)
To: Igor Grinberg; +Cc: Archit Taneja, linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <511BB113.3020108@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]
On 2013-02-13 17:28, Tomi Valkeinen wrote:
> On 2013-02-13 17:16, Igor Grinberg wrote:
>> Hi Archit,
>>
>> On 02/13/13 16:21, Archit Taneja wrote:
>>> The cm-t35 board file currently requests gpios required to configure the tdo35s
>>> panel, and provides platform_enable/disable callbacks to configure them.
>>>
>>> These tasks have been moved to the generic dpi panel driver itself and shouldn't
>>> be done in the board files.
>>>
>>> Remove the gpio requests and the platform callbacks from the board file.
>>> Add the gpio information to generic dpi panel's platform data so that it's
>>> passed to the panel driver.
>>>
>>> Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
>>> enabled after a 50 millisecond delay. This code has been removed and is not
>>> taken care of in the generic panel driver. The impact of this needs to be
>>> tested. The panel's gpios are also not exported any more. Hence, they can't be
>>> accessed via sysfs interface.
>>
>> Indeed, there is an impact - the LCD no longer works.
>> The reason for the LCD_EN gpio being pushed high after the 50ms delay,
>> is to get the LCD out of reset, so the SPI transaction will succeed
>> and initialize the LCD.
>> Now, when you remove the gpio handling for the LCD_EN pin,
>> the LCD no longer works.
>
> So between what is the sleep done? It's not clear from the code. LCD_EN
> needs to be 0 for 50ms, or...?
>
> If the panel requires specific reset handling, does it work right even
> currently when the panel is turned off and later turned on? The msleep
> is only used at boot time.
Okay, so I just realized there's an spi backlight driver used here, and
that backlight driver is actually handling the SPI transactions with the
panel, instead of the panel driver. So this looks quite messed up.
For a quick solution, can we just set the LCD_EN at boot time (with the
msleep), and not touch it after that?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 05/33] arm: omap: board-cm-t35: use generic dpi panel's gpio handling
From: Tomi Valkeinen @ 2013-02-13 15:28 UTC (permalink / raw)
To: Igor Grinberg; +Cc: Archit Taneja, linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <511BAE50.2090507@compulab.co.il>
[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]
On 2013-02-13 17:16, Igor Grinberg wrote:
> Hi Archit,
>
> On 02/13/13 16:21, Archit Taneja wrote:
>> The cm-t35 board file currently requests gpios required to configure the tdo35s
>> panel, and provides platform_enable/disable callbacks to configure them.
>>
>> These tasks have been moved to the generic dpi panel driver itself and shouldn't
>> be done in the board files.
>>
>> Remove the gpio requests and the platform callbacks from the board file.
>> Add the gpio information to generic dpi panel's platform data so that it's
>> passed to the panel driver.
>>
>> Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
>> enabled after a 50 millisecond delay. This code has been removed and is not
>> taken care of in the generic panel driver. The impact of this needs to be
>> tested. The panel's gpios are also not exported any more. Hence, they can't be
>> accessed via sysfs interface.
>
> Indeed, there is an impact - the LCD no longer works.
> The reason for the LCD_EN gpio being pushed high after the 50ms delay,
> is to get the LCD out of reset, so the SPI transaction will succeed
> and initialize the LCD.
> Now, when you remove the gpio handling for the LCD_EN pin,
> the LCD no longer works.
So between what is the sleep done? It's not clear from the code. LCD_EN
needs to be 0 for 50ms, or...?
If the panel requires specific reset handling, does it work right even
currently when the panel is turned off and later turned on? The msleep
is only used at boot time.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 05/33] arm: omap: board-cm-t35: use generic dpi panel's gpio handling
From: Igor Grinberg @ 2013-02-13 15:16 UTC (permalink / raw)
To: Archit Taneja; +Cc: tomi.valkeinen, linux-omap, linux-fbdev, Tony Lindgren
In-Reply-To: <1360765345-19312-6-git-send-email-archit@ti.com>
Hi Archit,
On 02/13/13 16:21, Archit Taneja wrote:
> The cm-t35 board file currently requests gpios required to configure the tdo35s
> panel, and provides platform_enable/disable callbacks to configure them.
>
> These tasks have been moved to the generic dpi panel driver itself and shouldn't
> be done in the board files.
>
> Remove the gpio requests and the platform callbacks from the board file.
> Add the gpio information to generic dpi panel's platform data so that it's
> passed to the panel driver.
>
> Note: In cm_t35_init_display(), the gpios were disabled, and the LCD_EN gpio was
> enabled after a 50 millisecond delay. This code has been removed and is not
> taken care of in the generic panel driver. The impact of this needs to be
> tested. The panel's gpios are also not exported any more. Hence, they can't be
> accessed via sysfs interface.
Indeed, there is an impact - the LCD no longer works.
The reason for the LCD_EN gpio being pushed high after the 50ms delay,
is to get the LCD out of reset, so the SPI transaction will succeed
and initialize the LCD.
Now, when you remove the gpio handling for the LCD_EN pin,
the LCD no longer works.
I don't agree with this breakage.
>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> arch/arm/mach-omap2/board-cm-t35.c | 46 ++++--------------------------------
> 1 file changed, 5 insertions(+), 41 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
> index f940a79..563d5ab 100644
> --- a/arch/arm/mach-omap2/board-cm-t35.c
> +++ b/arch/arm/mach-omap2/board-cm-t35.c
> @@ -189,32 +189,6 @@ static inline void cm_t35_init_nand(void) {}
> #define CM_T35_LCD_BL_GPIO 58
> #define CM_T35_DVI_EN_GPIO 54
>
> -static int lcd_enabled;
> -static int dvi_enabled;
> -
> -static int cm_t35_panel_enable_lcd(struct omap_dss_device *dssdev)
> -{
> - if (dvi_enabled) {
> - printk(KERN_ERR "cannot enable LCD, DVI is enabled\n");
> - return -EINVAL;
> - }
> -
> - gpio_set_value(CM_T35_LCD_EN_GPIO, 1);
> - gpio_set_value(CM_T35_LCD_BL_GPIO, 1);
> -
> - lcd_enabled = 1;
> -
> - return 0;
> -}
> -
> -static void cm_t35_panel_disable_lcd(struct omap_dss_device *dssdev)
> -{
> - lcd_enabled = 0;
> -
> - gpio_set_value(CM_T35_LCD_BL_GPIO, 0);
> - gpio_set_value(CM_T35_LCD_EN_GPIO, 0);
> -}
> -
> static int cm_t35_panel_enable_tv(struct omap_dss_device *dssdev)
> {
> return 0;
> @@ -226,8 +200,11 @@ static void cm_t35_panel_disable_tv(struct omap_dss_device *dssdev)
>
> static struct panel_generic_dpi_data lcd_panel = {
> .name = "toppoly_tdo35s",
> - .platform_enable = cm_t35_panel_enable_lcd,
> - .platform_disable = cm_t35_panel_disable_lcd,
> + .num_gpios = 2,
> + .gpios = {
> + CM_T35_LCD_BL_GPIO,
> + CM_T35_LCD_EN_GPIO
> + },
> };
>
> static struct omap_dss_device cm_t35_lcd_device = {
> @@ -303,19 +280,6 @@ static void __init cm_t35_init_display(void)
> spi_register_board_info(cm_t35_lcd_spi_board_info,
> ARRAY_SIZE(cm_t35_lcd_spi_board_info));
>
> - err = gpio_request_array(cm_t35_dss_gpios,
> - ARRAY_SIZE(cm_t35_dss_gpios));
> - if (err) {
> - pr_err("CM-T35: failed to request DSS control GPIOs\n");
> - return;
> - }
> -
> - gpio_export(CM_T35_LCD_EN_GPIO, 0);
> - gpio_export(CM_T35_LCD_BL_GPIO, 0);
> -
> - msleep(50);
> - gpio_set_value(CM_T35_LCD_EN_GPIO, 1);
> -
> err = omap_display_init(&cm_t35_dss_data);
> if (err) {
> pr_err("CM-T35: failed to register DSS device\n");
>
--
Regards,
Igor.
^ permalink raw reply
* [PATCH 33/33] OMAPDSS: remove reset_gpio field from omap_dss_device
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
The reset_gpio field isn't used by any panel driver to retrieve a reset gpio
number. All the panel drivers receive gpio data from their corresponding
platform_data structs. Remove the reset_gpio field.
Signed-off-by: Archit Taneja <archit@ti.com>
---
include/video/omapdss.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index e1ebb48..fe8672c 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -629,8 +629,6 @@ struct omap_dss_device {
struct rfbi_timings rfbi_timings;
} ctrl;
- int reset_gpio;
-
const char *name;
/* used to match device to driver */
--
1.7.9.5
^ permalink raw reply related
* [PATCH 32/33] arm: dss-common: don't use reset_gpio from omap4_panda_dvi_device
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja, Tony Lindgren
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
gpio reset info is passed to the tfp410 panel driver via the panel's platform
data struct 'tfp410_platform_data'. The tfp driver doesn't use the reset_gpio
field in the omap4_panda_dvi_device struct. Remove this field.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
arch/arm/mach-omap2/dss-common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-omap2/dss-common.c b/arch/arm/mach-omap2/dss-common.c
index b073e8b..393aeef 100644
--- a/arch/arm/mach-omap2/dss-common.c
+++ b/arch/arm/mach-omap2/dss-common.c
@@ -52,7 +52,6 @@ static struct omap_dss_device omap4_panda_dvi_device = {
.driver_name = "tfp410",
.data = &omap4_dvi_panel,
.phy.dpi.data_lines = 24,
- .reset_gpio = PANDA_DVI_TFP410_POWER_DOWN_GPIO,
.channel = OMAP_DSS_CHANNEL_LCD2,
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 31/33] OMAPDSS: remove platform_enable/disable callbacks from omap_dss_device
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
None of the omapdss panel drivers call platform_enable/disable callbacks, and
none of the omap board files define these callbacks for any omap_dss_device.
Hence these callbacks can be removed form the omap_dss_device struct.
Signed-off-by: Archit Taneja <archit@ti.com>
---
include/video/omapdss.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index f0b65a5..e1ebb48 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -650,10 +650,6 @@ struct omap_dss_device {
enum omap_dss_display_state state;
enum omap_dss_audio_state audio_state;
-
- /* platform specific */
- int (*platform_enable)(struct omap_dss_device *dssdev);
- void (*platform_disable)(struct omap_dss_device *dssdev);
};
struct omap_dss_hdmi_data
--
1.7.9.5
^ permalink raw reply related
* [PATCH 30/33] OMAPDSS: VENC: remove platform_enable/disable calls
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
The platform_enable/disable callbacks in board files for VENC omap_dss_device
instances don't do anything. Hence, we can remove these callbacks from the VENC
driver.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/dss/venc.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 006caf3..1598d27 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -519,10 +519,6 @@ int omapdss_venc_display_enable(struct omap_dss_device *dssdev)
goto err0;
}
- if (dssdev->platform_enable)
- dssdev->platform_enable(dssdev);
-
-
r = venc_power_on(dssdev);
if (r)
goto err1;
@@ -533,8 +529,6 @@ int omapdss_venc_display_enable(struct omap_dss_device *dssdev)
return 0;
err1:
- if (dssdev->platform_disable)
- dssdev->platform_disable(dssdev);
omap_dss_stop_device(dssdev);
err0:
mutex_unlock(&venc.venc_lock);
@@ -551,9 +545,6 @@ void omapdss_venc_display_disable(struct omap_dss_device *dssdev)
omap_dss_stop_device(dssdev);
- if (dssdev->platform_disable)
- dssdev->platform_disable(dssdev);
-
mutex_unlock(&venc.venc_lock);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 29/33] arm: omap boards: Remove unnecessary platform_enable/disable callbacks for VENC device
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja, Tony Lindgren
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
The omap_dss_device's platform_enable/disable callbacks don't do anything for
any of the boards. The platform calls from the VENC driver will also be removed
in the future. Remove these calls from the board which have a VENC device.
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
arch/arm/mach-omap2/board-3430sdp.c | 11 -----------
arch/arm/mach-omap2/board-am3517evm.c | 11 -----------
arch/arm/mach-omap2/board-cm-t35.c | 11 -----------
arch/arm/mach-omap2/board-omap3evm.c | 11 -----------
arch/arm/mach-omap2/board-omap3stalker.c | 11 -----------
5 files changed, 55 deletions(-)
diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 13f1431..717dd15 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -124,15 +124,6 @@ static void __init sdp3430_display_init(void)
}
-static int sdp3430_panel_enable_tv(struct omap_dss_device *dssdev)
-{
- return 0;
-}
-
-static void sdp3430_panel_disable_tv(struct omap_dss_device *dssdev)
-{
-}
-
static struct panel_sharp_ls037v7dw01_data sdp3430_lcd_data = {
.resb_gpio = SDP3430_LCD_PANEL_ENABLE_GPIO,
.ini_gpio = -1,
@@ -167,8 +158,6 @@ static struct omap_dss_device sdp3430_tv_device = {
.driver_name = "venc",
.type = OMAP_DISPLAY_TYPE_VENC,
.phy.venc.type = OMAP_DSS_VENC_TYPE_SVIDEO,
- .platform_enable = sdp3430_panel_enable_tv,
- .platform_disable = sdp3430_panel_disable_tv,
};
diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
index eb245a6..edb4cce 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -138,22 +138,11 @@ static struct omap_dss_device am3517_evm_lcd_device = {
.phy.dpi.data_lines = 16,
};
-static int am3517_evm_panel_enable_tv(struct omap_dss_device *dssdev)
-{
- return 0;
-}
-
-static void am3517_evm_panel_disable_tv(struct omap_dss_device *dssdev)
-{
-}
-
static struct omap_dss_device am3517_evm_tv_device = {
.type = OMAP_DISPLAY_TYPE_VENC,
.name = "tv",
.driver_name = "venc",
.phy.venc.type = OMAP_DSS_VENC_TYPE_SVIDEO,
- .platform_enable = am3517_evm_panel_enable_tv,
- .platform_disable = am3517_evm_panel_disable_tv,
};
static struct tfp410_platform_data dvi_panel = {
diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
index 563d5ab..cb3b078 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -189,15 +189,6 @@ static inline void cm_t35_init_nand(void) {}
#define CM_T35_LCD_BL_GPIO 58
#define CM_T35_DVI_EN_GPIO 54
-static int cm_t35_panel_enable_tv(struct omap_dss_device *dssdev)
-{
- return 0;
-}
-
-static void cm_t35_panel_disable_tv(struct omap_dss_device *dssdev)
-{
-}
-
static struct panel_generic_dpi_data lcd_panel = {
.name = "toppoly_tdo35s",
.num_gpios = 2,
@@ -233,8 +224,6 @@ static struct omap_dss_device cm_t35_tv_device = {
.driver_name = "venc",
.type = OMAP_DISPLAY_TYPE_VENC,
.phy.venc.type = OMAP_DSS_VENC_TYPE_SVIDEO,
- .platform_enable = cm_t35_panel_enable_tv,
- .platform_disable = cm_t35_panel_disable_tv,
};
static struct omap_dss_device *cm_t35_dss_devices[] = {
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index dfb5e1b..a471476 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -201,22 +201,11 @@ static struct omap_dss_device omap3_evm_lcd_device = {
.data = &omap3_evm_lcd_data,
};
-static int omap3_evm_enable_tv(struct omap_dss_device *dssdev)
-{
- return 0;
-}
-
-static void omap3_evm_disable_tv(struct omap_dss_device *dssdev)
-{
-}
-
static struct omap_dss_device omap3_evm_tv_device = {
.name = "tv",
.driver_name = "venc",
.type = OMAP_DISPLAY_TYPE_VENC,
.phy.venc.type = OMAP_DSS_VENC_TYPE_SVIDEO,
- .platform_enable = omap3_evm_enable_tv,
- .platform_disable = omap3_evm_disable_tv,
};
static struct tfp410_platform_data dvi_panel = {
diff --git a/arch/arm/mach-omap2/board-omap3stalker.c b/arch/arm/mach-omap2/board-omap3stalker.c
index 621ace1..8f1b0a1 100644
--- a/arch/arm/mach-omap2/board-omap3stalker.c
+++ b/arch/arm/mach-omap2/board-omap3stalker.c
@@ -93,15 +93,6 @@ static void __init omap3_stalker_display_init(void)
return;
}
-static int omap3_stalker_enable_tv(struct omap_dss_device *dssdev)
-{
- return 0;
-}
-
-static void omap3_stalker_disable_tv(struct omap_dss_device *dssdev)
-{
-}
-
static struct omap_dss_device omap3_stalker_tv_device = {
.name = "tv",
.driver_name = "venc",
@@ -111,8 +102,6 @@ static struct omap_dss_device omap3_stalker_tv_device = {
#elif defined(CONFIG_OMAP2_VENC_OUT_TYPE_COMPOSITE)
.u.venc.type = OMAP_DSS_VENC_TYPE_COMPOSITE,
#endif
- .platform_enable = omap3_stalker_enable_tv,
- .platform_disable = omap3_stalker_disable_tv,
};
static struct tfp410_platform_data dvi_panel = {
--
1.7.9.5
^ permalink raw reply related
* [PATCH 28/33] OMAPDSS: n8x0 panel: remove platform_enable/disable callbacks
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
The n8x0 panel driver now manages the gpios required to configure the panel.
This was previously done in panel_n8x0_data's platform_enable/disable callbacks
defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_n8x0_data struct, which is needed by the panel driver to
configure the gpios connected to the panel. Hence, the platform_enable/disable
ops can be safely removed now.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/displays/panel-n8x0.c | 12 ------------
include/video/omap-panel-data.h | 2 --
2 files changed, 14 deletions(-)
diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
index c146a3d..f94ead6 100644
--- a/drivers/video/omap2/displays/panel-n8x0.c
+++ b/drivers/video/omap2/displays/panel-n8x0.c
@@ -295,12 +295,6 @@ static int n8x0_panel_power_on(struct omap_dss_device *dssdev)
gpio_direction_output(bdata->ctrl_pwrdown, 1);
- if (bdata->platform_enable) {
- r = bdata->platform_enable(dssdev);
- if (r)
- goto err_plat_en;
- }
-
omapdss_rfbi_set_size(dssdev, dssdev->panel.timings.x_res,
dssdev->panel.timings.y_res);
omapdss_rfbi_set_pixel_size(dssdev, dssdev->ctrl.pixel_size);
@@ -373,9 +367,6 @@ err_inv_panel:
err_inv_chip:
omapdss_rfbi_display_disable(dssdev);
err_rfbi_en:
- if (bdata->platform_disable)
- bdata->platform_disable(dssdev);
-err_plat_en:
gpio_direction_output(bdata->ctrl_pwrdown, 0);
return r;
}
@@ -392,9 +383,6 @@ static void n8x0_panel_power_off(struct omap_dss_device *dssdev)
send_display_off(spi);
send_sleep_in(spi);
- if (bdata->platform_disable)
- bdata->platform_disable(dssdev);
-
/*
* HACK: we should turn off the panel here, but there is some problem
* with the initialization sequence, and we fail to init the panel if we
diff --git a/include/video/omap-panel-data.h b/include/video/omap-panel-data.h
index 7cac89e..84f332e 100644
--- a/include/video/omap-panel-data.h
+++ b/include/video/omap-panel-data.h
@@ -48,8 +48,6 @@ struct panel_generic_dpi_data {
* struct panel_n8x0_data - N800 panel driver configuration data
*/
struct panel_n8x0_data {
- int (*platform_enable)(struct omap_dss_device *dssdev);
- void (*platform_disable)(struct omap_dss_device *dssdev);
int panel_reset;
int ctrl_pwrdown;
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH 27/33] OMAPDSS: n8x0 panel: handle gpio data in panel driver
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
The n8x0 panel driver leaves gpio configurations to the platform_enable and
disable calls in the platform's board file. These should happen in the panel
driver itself.
A platform data struct called panel_n8x0_data already exists to hold gpio
numbers and other platform data. However, the gpio requests are expected to be
done in the board file and not the panel driver.
Request all the gpios in the panel driver so that the board files which use
the the panel don't need to do it. This will help in removing the need for the
panel drivers to have platform related callbacks.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/displays/panel-n8x0.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
index 9c55c91..c146a3d 100644
--- a/drivers/video/omap2/displays/panel-n8x0.c
+++ b/drivers/video/omap2/displays/panel-n8x0.c
@@ -426,6 +426,7 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
{
struct panel_n8x0_data *bdata = get_board_data(dssdev);
struct panel_drv_data *ddata;
+ int r;
dev_dbg(&dssdev->dev, "probe\n");
@@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
+ if (gpio_is_valid(bdata->panel_reset)) {
+ r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
+ GPIOF_OUT_INIT_LOW, "PANEL RESET");
+ if (r)
+ return r;
+ }
+
+ if (gpio_is_valid(bdata->ctrl_pwrdown)) {
+ r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
+ GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
+ if (r)
+ return r;
+ }
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 26/33] OMAPDSS: picodlp panel: remove platform_enable/disable callbacks
From: Archit Taneja @ 2013-02-13 14:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1360765345-19312-1-git-send-email-archit@ti.com>
The picodlp panel driver now manages the gpios required to configure the
panel. This was previously done in omap_dss_device's platform_enable/disable
callbacks defined in board files using this panel.
All the board files using this panel now pass the gpio information as platform
data via the panel_generic_dpi_data struct, which is needed by the panel driver
to configure the gpios connected to the panel. Hence, the
platform_enable/disable ops can be safely removed now.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/video/omap2/displays/panel-picodlp.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
index bc8ac77..62f2db0 100644
--- a/drivers/video/omap2/displays/panel-picodlp.c
+++ b/drivers/video/omap2/displays/panel-picodlp.c
@@ -354,12 +354,6 @@ static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
- if (dssdev->platform_enable) {
- r = dssdev->platform_enable(dssdev);
- if (r)
- return r;
- }
-
gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
msleep(1);
gpio_set_value(picodlp_pdata->pwrgood_gpio, 1);
@@ -398,9 +392,6 @@ static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
err:
omapdss_dpi_display_disable(dssdev);
err1:
- if (dssdev->platform_disable)
- dssdev->platform_disable(dssdev);
-
return r;
}
@@ -412,9 +403,6 @@ static void picodlp_panel_power_off(struct omap_dss_device *dssdev)
gpio_set_value(picodlp_pdata->emu_done_gpio, 0);
gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
-
- if (dssdev->platform_disable)
- dssdev->platform_disable(dssdev);
}
static int picodlp_panel_probe(struct omap_dss_device *dssdev)
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox