Re: Patch "REMOVE OMAP LEGACY CODE: Reset mach-omap1/board-*.c files to mainline" breaks nokia770

[Tony Lindgren <tony@atomide.com>]

* Andrew de Quincey <adq_dvb@lidskialf.net> [090514 16:19]:
> Quoting Tony Lindgren <tony@atomide.com>:
>
>> * Felipe Balbi <felipe.balbi@nokia.com> [090514 01:15]:
>>> On Thu, May 14, 2009 at 03:44:14AM +0200, ext Tony Lindgren wrote:
>>> > * Felipe Balbi <felipe.balbi@nokia.com> [090513 17:33]:
>>> > > On Thu, May 14, 2009 at 01:46:51AM +0200, ext Andrew de Quincey wrote:
>>> > > > Hi, I've just discovered that the patch at:
>>> > > >
>>> > > >  
>>> http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commitdiff;h=3eae3ea7c443fc4330574dffea65b6f2f53a2574
>>> > > >
>>> > > > Breaks the nokia770's framebuffer as it removes the platform data for
>>> > > > the HWA742 LCD controller.
>>> > > >
>>> > > > As the patch says "Patches against the mainline tree are welcome to
>>> > > > add back the missing functionality if needed!", I'm happy to do this.
>>> > > >
>>> > > > However, since I'm fairly new to the linux-omap project, is simply
>>> > > > extracting the removed nokia770 code and generating a patch against
>>> > > > the mainline kernel sufficient? or is there a newer style of some sort
>>> > > > that should be adopted for this?
>>> > >
>>> > > You can start by generating the new patch against mainline and running
>>> > > scripts/checkpatch.pl, then you should probably find out if there are
>>> > > any API changes and stuff like that. Then send the patch to lkml ccing
>>> > > linux-omap and let's see what comments do you get from those guys :-)
>>> >
>>> > Please change the code to pass the struct clock to  
>>> drivers/video/omap/hwa742.c
>>> > in hwa742_platform_data. The hw742.c can just do standard  
>>> clk_enable/disable
>>> > there. Otherwise we won't be able to get this missing part to the mainline
>>> > kernel.
>>>
>>> how about clkdev then ? are we gonna support that for omap1 still ?
>>
>> Yeah the clkdev is there, but in the case of the LCD the clock can be
>> any clock, so we cannot set just one "fck" for all the omap1 LCD panels.
>>
>> In this case the "bclk" is used for the LCD, but "bclk" could be used
>> for other devices as well, not just LCD.
>
> Hi, is the attached patch the sort of thing you had in mind? This is  
> against linux-omap-2.6, latest git.

Hey, that's pretty cool! Just one comment below.

> Unfortunately, both mainline linux-2.6 and linux-omap-2.6 gits lock up  
> on bootup before the LCD is initialised. However, the linux-omap 2.6.29 
> that is compiled by openembedded still boots ok, and with the attached 
> patch applied, the LCD works again!

OK, good to hear. Need to debug that then.

> I guess my next task is to figure out why the mainline kernels freeze;  
> I've seen that before, and it seemed to be something to do with changes 
> to the mcbsp support.

OK

> Reinstate HWA742 platform data for nokia 770 platform.
> 
> Signed-off-by: Andrew de Quincey <adq_dvb@lidskialf.net>
> 
> diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
> index 8780ca6..df5f38b 100644
> --- a/arch/arm/mach-omap1/board-nokia770.c
> +++ b/arch/arm/mach-omap1/board-nokia770.c
> @@ -33,6 +33,7 @@
>  #include <mach/common.h>
>  #include <mach/dsp_common.h>
>  #include <mach/omapfb.h>
> +#include <mach/hwa742.h>
>  #include <mach/lcd_mipid.h>
>  #include <mach/mmc.h>
>  #include <mach/usb.h>
> @@ -163,6 +164,26 @@ static struct spi_board_info nokia770_spi_board_info[] __initdata = {
>  	},
>  };
>  
> +static struct hwa742_platform_data nokia770_hwa742_platform_data = {
> +	.sys_ck 		= NULL,
> +	.te_connected		= 1,
> +};
> +
> +static int hwa742_get_clocks(void)
> +{
> +	nokia770_hwa742_platform_data.sys_ck = clk_get(NULL, "bclk");
> +	if (IS_ERR(nokia770_hwa742_platform_data.sys_ck)) {
> +		printk(KERN_ERR "can't get HWA742 clock\n");
> +		return PTR_ERR(nokia770_hwa742_platform_data.sys_ck);
> +	}
> +	return 0;
> +}
> +
> +static void hwa742_dev_init(void)
> +{
> +	hwa742_get_clocks();
> +	omapfb_set_ctrl_platform_data(&nokia770_hwa742_platform_data);
> +}

You can now get rid of the hwa742_get_clocks(), and move that code
to hwa742_dev_init() instead.

Regards,

Tony

  
>  /* assume no Mini-AB port */
>  
> @@ -371,6 +392,7 @@ static void __init omap_nokia770_init(void)
>  	omap_serial_init();
>  	omap_register_i2c_bus(1, 100, NULL, 0);
>  	omap_dsp_init();
> +	hwa742_dev_init();
>  	ads7846_dev_init();
>  	mipid_dev_init();
>  	omap_usb_init(&nokia770_usb_config);
> diff --git a/arch/arm/plat-omap/include/mach/hwa742.h b/arch/arm/plat-omap/include/mach/hwa742.h
> index 577f492..c00e05d 100644
> --- a/arch/arm/plat-omap/include/mach/hwa742.h
> +++ b/arch/arm/plat-omap/include/mach/hwa742.h
> @@ -2,10 +2,7 @@
>  #define _HWA742_H
>  
>  struct hwa742_platform_data {
> -	void		(*power_up)(struct device *dev);
> -	void		(*power_down)(struct device *dev);
> -	unsigned long	(*get_clock_rate)(struct device *dev);
> -
> +	struct clk 	*sys_ck;
>  	unsigned	te_connected:1;
>  };
>  
> diff --git a/drivers/video/omap/hwa742.c b/drivers/video/omap/hwa742.c
> index 8aa6e47..1230476 100644
> --- a/drivers/video/omap/hwa742.c
> +++ b/drivers/video/omap/hwa742.c
> @@ -133,8 +133,7 @@ struct {
>  	struct lcd_ctrl_extif	*extif;
>  	struct lcd_ctrl		*int_ctrl;
>  
> -	void			(*power_up)(struct device *dev);
> -	void			(*power_down)(struct device *dev);
> +	struct clk		*sys_ck;
>  } hwa742;
>  
>  struct lcd_ctrl hwa742_ctrl;
> @@ -915,14 +914,13 @@ static void hwa742_suspend(void)
>  	hwa742_set_update_mode(OMAPFB_UPDATE_DISABLED);
>  	/* Enable sleep mode */
>  	hwa742_write_reg(HWA742_POWER_SAVE, 1 << 1);
> -	if (hwa742.power_down != NULL)
> -		hwa742.power_down(hwa742.fbdev->dev);
> +	clk_disable(hwa742.sys_ck);
>  }
>  
>  static void hwa742_resume(void)
>  {
> -	if (hwa742.power_up != NULL)
> -		hwa742.power_up(hwa742.fbdev->dev);
> +	clk_enable(hwa742.sys_ck);
> +
>  	/* Disable sleep mode */
>  	hwa742_write_reg(HWA742_POWER_SAVE, 0);
>  	while (1) {
> @@ -955,14 +953,13 @@ static int hwa742_init(struct omapfb_device *fbdev, int ext_mode,
>  	omapfb_conf = fbdev->dev->platform_data;
>  	ctrl_conf = omapfb_conf->ctrl_platform_data;
>  
> -	if (ctrl_conf == NULL || ctrl_conf->get_clock_rate == NULL) {
> +	if (ctrl_conf == NULL || ctrl_conf->sys_ck == NULL) {
>  		dev_err(fbdev->dev, "HWA742: missing platform data\n");
>  		r = -ENOENT;
>  		goto err1;
>  	}
>  
> -	hwa742.power_down = ctrl_conf->power_down;
> -	hwa742.power_up = ctrl_conf->power_up;
> +	hwa742.sys_ck = ctrl_conf->sys_ck;
>  
>  	spin_lock_init(&hwa742.req_lock);
>  
> @@ -972,12 +969,11 @@ static int hwa742_init(struct omapfb_device *fbdev, int ext_mode,
>  	if ((r = hwa742.extif->init(fbdev)) < 0)
>  		goto err2;
>  
> -	ext_clk = ctrl_conf->get_clock_rate(fbdev->dev);
> +	ext_clk = clk_get_rate(hwa742.sys_ck);
>  	if ((r = calc_extif_timings(ext_clk, &extif_mem_div)) < 0)
>  		goto err3;
>  	hwa742.extif->set_timings(&hwa742.reg_timings);
> -	if (hwa742.power_up != NULL)
> -		hwa742.power_up(fbdev->dev);
> +	clk_enable(hwa742.sys_ck);
>  
>  	calc_hwa742_clk_rates(ext_clk, &sys_clk, &pix_clk);
>  	if ((r = calc_extif_timings(sys_clk, &extif_mem_div)) < 0)
> @@ -1040,8 +1036,7 @@ static int hwa742_init(struct omapfb_device *fbdev, int ext_mode,
>  
>  	return 0;
>  err4:
> -	if (hwa742.power_down != NULL)
> -		hwa742.power_down(fbdev->dev);
> +	clk_disable(hwa742.sys_ck);
>  err3:
>  	hwa742.extif->cleanup();
>  err2:
> @@ -1055,8 +1050,7 @@ static void hwa742_cleanup(void)
>  	hwa742_set_update_mode(OMAPFB_UPDATE_DISABLED);
>  	hwa742.extif->cleanup();
>  	hwa742.int_ctrl->cleanup();
> -	if (hwa742.power_down != NULL)
> -		hwa742.power_down(hwa742.fbdev->dev);
> +	clk_disable(hwa742.sys_ck);
>  }
>  
>  struct lcd_ctrl hwa742_ctrl = {