* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Arnd Bergmann @ 2013-03-20 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHkwnC-8FH0nyJ+eT=+7doP+fSdZjNYUW4zzs_r6e9wt3Yt4Fg@mail.gmail.com>
On Wednesday 20 March 2013, Fabio Porcedda wrote:
>
> On Wed, Mar 20, 2013 at 11:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 20 March 2013, Fabio Porcedda wrote:
> >> I think we can check inside the deferred_probe_work_func()
> >> if the dev->probe function pointer is equal to platform_drv_probe_fail().
> >
> > I think it's too late by then, because that would only warn if we try to probe
> > it again, but when platform_driver_probe() does not succeed immediately, it
>
> Maybe you mean "does succeed immediately" ?
I mean in this code (simplified for the sake of discussion)
int __init_or_module platform_driver_probe(struct platform_driver *drv,
int (*probe)(struct platform_device *))
{
int retval, code;
drv->probe = probe;
retval = code = platform_driver_register(drv);
drv->probe = NULL;
if (code = 0 && list_empty(&drv->driver.p->klist_devices.k_list))
retval = -ENODEV;
drv->driver.probe = platform_drv_probe_fail;
if (code != retval)
platform_driver_unregister(drv);
return retval;
}
we assume that all devices are bound to drivers during the call to
platform_driver_register, and if the device list is empty afterwards,
we unregister the driver and will never get to the deferred probing
stage.
Arnd
^ permalink raw reply
* Re: [PATCH 03/14] OMAPDSS: DSI: simplify dsi configuration
From: Tomi Valkeinen @ 2013-03-20 11:44 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <51499C73.3060802@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]
On 2013-03-20 13:24, Archit Taneja wrote:
> On Friday 08 March 2013 05:22 PM, Tomi Valkeinen wrote:
>> We have a bunch of dsi functions that are used to do the basic
>> configuration for DSI. To simplify things, and to make sure we have all
>> the necessary information, create a single dsi config function, which
>> does the basic configuration.
>
> I had split these funcs in the manner so that they could be converted
> into generic output ops or something equivalent to what we anticipated
> CDF to represent encoders. Hence, we may have to split this into smaller
> funcs again later :p
Well, it was from the CDF discussions that this change arose. Everybody
wanted a simpler way than n+1 functions.
And I think it makes sense. It makes it possible to manage the
configuration as one "whole", instead of small bits that may have
interdependencies. E.g the size of the output affects video mode
calculations, so one had to make the calls in certain order. Now we have
all the needed information in one piece.
We could, perhaps, have common parts between different video busses, but
I'm not sure if configuration is one of those common parts.
> Also, set_size and set_timings were 2 different ops for command and
> video mode panels respectively. omapdss_dsi_set_size() also came in use
> when we supported rotation in Taal. We have an equivalent func for rfbi.
Yep. I felt it's a bit confusing, so I just combined them. Even for
command mode some timing information is good (well, pixel clock), to
calculate proper DSI bus speed.
I think this also works in case of panel rotation. From DSS's point of
view (and that's what we're talking about when setting the timings)
there's no rotation. It's the panel's internal thing.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 05/14] OMAPDSS: DSI: add enum omap_dss_dsi_trans_mode
From: Archit Taneja @ 2013-03-20 11:42 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743569-10289-6-git-send-email-tomi.valkeinen@ti.com>
On Friday 08 March 2013 05:22 PM, Tomi Valkeinen wrote:
> Instead of managing DSI sync ends with booleans, add an enum for the DSI
> transfer mode. This is much cleaner way to handle the DSI syncs.
This sort of DSI protocol specific enums should definitely get into the
CDF encoder drivers, maybe even in include/video/mipi_display.h for the
time CDF is being worked upon.
Archit
^ permalink raw reply
* Re: [PATCH 03/14] OMAPDSS: DSI: simplify dsi configuration
From: Archit Taneja @ 2013-03-20 11:36 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743569-10289-4-git-send-email-tomi.valkeinen@ti.com>
On Friday 08 March 2013 05:22 PM, Tomi Valkeinen wrote:
> We have a bunch of dsi functions that are used to do the basic
> configuration for DSI. To simplify things, and to make sure we have all
> the necessary information, create a single dsi config function, which
> does the basic configuration.
I had split these funcs in the manner so that they could be converted
into generic output ops or something equivalent to what we anticipated
CDF to represent encoders. Hence, we may have to split this into smaller
funcs again later :p
Also, set_size and set_timings were 2 different ops for command and
video mode panels respectively. omapdss_dsi_set_size() also came in use
when we supported rotation in Taal. We have an equivalent func for rfbi.
Archit
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/displays/panel-taal.c | 16 ++++---
> drivers/video/omap2/dss/dsi.c | 74 ++++-------------------------
> include/video/omapdss.h | 23 ++++-----
> 3 files changed, 31 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> index bc4c95e..eb86cba 100644
> --- a/drivers/video/omap2/displays/panel-taal.c
> +++ b/drivers/video/omap2/displays/panel-taal.c
> @@ -919,6 +919,13 @@ static int taal_power_on(struct omap_dss_device *dssdev)
> struct taal_data *td = dev_get_drvdata(&dssdev->dev);
> u8 id1, id2, id3;
> int r;
> + struct omap_dss_dsi_config dsi_config = {
> + .mode = OMAP_DSS_DSI_CMD_MODE,
> + .pixel_format = OMAP_DSS_DSI_FMT_RGB888,
> + .timings = &dssdev->panel.timings,
> + .hs_clk = 216000000,
> + .lp_clk = 10000000,
> + };
>
> r = omapdss_dsi_configure_pins(dssdev, &td->pin_config);
> if (r) {
> @@ -926,14 +933,9 @@ static int taal_power_on(struct omap_dss_device *dssdev)
> goto err0;
> };
>
> - omapdss_dsi_set_size(dssdev, dssdev->panel.timings.x_res,
> - dssdev->panel.timings.y_res);
> - omapdss_dsi_set_pixel_format(dssdev, OMAP_DSS_DSI_FMT_RGB888);
> - omapdss_dsi_set_operation_mode(dssdev, OMAP_DSS_DSI_CMD_MODE);
> -
> - r = omapdss_dsi_set_clocks(dssdev, 216000000, 10000000);
> + r = omapdss_dsi_set_config(dssdev, &dsi_config);
> if (r) {
> - dev_err(&dssdev->dev, "failed to set HS and LP clocks\n");
> + dev_err(&dssdev->dev, "failed to configure DSI\n");
> goto err0;
> }
>
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 5f5b475..901b721 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -4278,7 +4278,7 @@ int omapdss_dsi_configure_pins(struct omap_dss_device *dssdev,
> }
> EXPORT_SYMBOL(omapdss_dsi_configure_pins);
>
> -int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
> +static int dsi_set_clocks(struct omap_dss_device *dssdev,
> unsigned long ddr_clk, unsigned long lp_clk)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> @@ -4293,8 +4293,6 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
>
> DSSDBG("Setting DSI clocks: ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
>
> - mutex_lock(&dsi->lock);
> -
> /* Calculate PLL output clock */
> r = dsi_pll_calc_ddrfreq(dsidev, ddr_clk * 4, &cinfo);
> if (r)
> @@ -4336,13 +4334,10 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
> OMAP_DSS_CLK_SRC_DSI_PLL_HSDIV_DSI :
> OMAP_DSS_CLK_SRC_DSI2_PLL_HSDIV_DSI;
>
> - mutex_unlock(&dsi->lock);
> return 0;
> err:
> - mutex_unlock(&dsi->lock);
> return r;
> }
> -EXPORT_SYMBOL(omapdss_dsi_set_clocks);
>
> int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)
> {
> @@ -4884,75 +4879,26 @@ int omapdss_dsi_enable_te(struct omap_dss_device *dssdev, bool enable)
> }
> EXPORT_SYMBOL(omapdss_dsi_enable_te);
>
> -void omapdss_dsi_set_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> +int omapdss_dsi_set_config(struct omap_dss_device *dssdev,
> + const struct omap_dss_dsi_config *config)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
>
> mutex_lock(&dsi->lock);
>
> - dsi->timings = *timings;
> -
> - mutex_unlock(&dsi->lock);
> -}
> -EXPORT_SYMBOL(omapdss_dsi_set_timings);
> -
> -void omapdss_dsi_set_size(struct omap_dss_device *dssdev, u16 w, u16 h)
> -{
> - struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> - struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> -
> - mutex_lock(&dsi->lock);
> + dsi->timings = *config->timings;
> + dsi->vm_timings = *config->vm_timings;
> + dsi->pix_fmt = config->pixel_format;
> + dsi->mode = config->mode;
>
> - dsi->timings.x_res = w;
> - dsi->timings.y_res = h;
> + dsi_set_clocks(dssdev, config->hs_clk, config->lp_clk);
>
> mutex_unlock(&dsi->lock);
> -}
> -EXPORT_SYMBOL(omapdss_dsi_set_size);
>
> -void omapdss_dsi_set_pixel_format(struct omap_dss_device *dssdev,
> - enum omap_dss_dsi_pixel_format fmt)
> -{
> - struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> - struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> -
> - mutex_lock(&dsi->lock);
> -
> - dsi->pix_fmt = fmt;
> -
> - mutex_unlock(&dsi->lock);
> -}
> -EXPORT_SYMBOL(omapdss_dsi_set_pixel_format);
> -
> -void omapdss_dsi_set_operation_mode(struct omap_dss_device *dssdev,
> - enum omap_dss_dsi_mode mode)
> -{
> - struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> - struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> -
> - mutex_lock(&dsi->lock);
> -
> - dsi->mode = mode;
> -
> - mutex_unlock(&dsi->lock);
> -}
> -EXPORT_SYMBOL(omapdss_dsi_set_operation_mode);
> -
> -void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
> - struct omap_dss_dsi_videomode_timings *timings)
> -{
> - struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> - struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> -
> - mutex_lock(&dsi->lock);
> -
> - dsi->vm_timings = *timings;
> -
> - mutex_unlock(&dsi->lock);
> + return 0;
> }
> -EXPORT_SYMBOL(omapdss_dsi_set_videomode_timings);
> +EXPORT_SYMBOL(omapdss_dsi_set_config);
>
> /*
> * Return a hardcoded channel for the DSI output. This should work for
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 9057e21..4a52197 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -282,6 +282,16 @@ struct omap_dss_dsi_videomode_timings {
> int window_sync;
> };
>
> +struct omap_dss_dsi_config {
> + enum omap_dss_dsi_mode mode;
> + enum omap_dss_dsi_pixel_format pixel_format;
> + const struct omap_video_timings *timings;
> + const struct omap_dss_dsi_videomode_timings *vm_timings;
> +
> + unsigned long hs_clk;
> + unsigned long lp_clk;
> +};
> +
> void dsi_bus_lock(struct omap_dss_device *dssdev);
> void dsi_bus_unlock(struct omap_dss_device *dssdev);
> int dsi_vc_dcs_write(struct omap_dss_device *dssdev, int channel, u8 *data,
> @@ -805,15 +815,8 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
> void omapdss_dsi_vc_enable_hs(struct omap_dss_device *dssdev, int channel,
> bool enable);
> int omapdss_dsi_enable_te(struct omap_dss_device *dssdev, bool enable);
> -void omapdss_dsi_set_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings);
> -void omapdss_dsi_set_size(struct omap_dss_device *dssdev, u16 w, u16 h);
> -void omapdss_dsi_set_pixel_format(struct omap_dss_device *dssdev,
> - enum omap_dss_dsi_pixel_format fmt);
> -void omapdss_dsi_set_operation_mode(struct omap_dss_device *dssdev,
> - enum omap_dss_dsi_mode mode);
> -void omapdss_dsi_set_videomode_timings(struct omap_dss_device *dssdev,
> - struct omap_dss_dsi_videomode_timings *timings);
> +int omapdss_dsi_set_config(struct omap_dss_device *dssdev,
> + const struct omap_dss_dsi_config *config);
>
> int omap_dsi_update(struct omap_dss_device *dssdev, int channel,
> void (*callback)(int, void *), void *data);
> @@ -822,8 +825,6 @@ int omap_dsi_set_vc_id(struct omap_dss_device *dssdev, int channel, int vc_id);
> void omap_dsi_release_vc(struct omap_dss_device *dssdev, int channel);
> int omapdss_dsi_configure_pins(struct omap_dss_device *dssdev,
> const struct omap_dsi_pin_config *pin_cfg);
> -int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
> - unsigned long ddr_clk, unsigned long lp_clk);
>
> int omapdss_dsi_display_enable(struct omap_dss_device *dssdev);
> void omapdss_dsi_display_disable(struct omap_dss_device *dssdev,
>
^ permalink raw reply
* Re: [PATCH 01/14] OMAPDSS: DISPC: store core clk rate
From: Tomi Valkeinen @ 2013-03-20 11:36 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <514998B1.8040907@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]
On 2013-03-20 13:08, Archit Taneja wrote:
> Hi,
>
> On Friday 08 March 2013 05:22 PM, Tomi Valkeinen wrote:
>> Store dispc core clock rate so that it's available for calculations even
>> if the HW is disabled.
>
> I think the core_clk_rate variable should change when we change the lcd
> clock source through dss_select_lcd_clk_source() for omap3.
>
> If we have the following sequence:
>
> ...
> dispc_mgr_set_lcd_divisor();
> dss_select_lcd_clk_source();
> ...
>
> The value of core_clock variable would be based on the previous clock
> source, and not the current one.
>
> This situation doesn't occur currently as the 'apply' framework delays
> all dispc writes to the point when we enable the manager. So the
> sequence above cannot occur. But maybe we should keep this in mind when
> we move more things to omapdrm, where 'apply' isn't in use.
Hmm. Good point.
I don't think this has to do with apply system. The clock source is set
by the output drivers, and the output drivers also calculate the
divisors, and call the functions to set the divisors. Both DPI and DSI
drivers first set the clock source, and then call the
dss_mgr_set_lcd_config() which sets the divisors (causing the recalc).
Whether using the apply or not, I think it should work correctly. But
it's clearly something that is a bit fragile.
*cough*commonclockframework*cough*.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH 01/14] OMAPDSS: DISPC: store core clk rate
From: Archit Taneja @ 2013-03-20 11:20 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1362743569-10289-2-git-send-email-tomi.valkeinen@ti.com>
Hi,
On Friday 08 March 2013 05:22 PM, Tomi Valkeinen wrote:
> Store dispc core clock rate so that it's available for calculations even
> if the HW is disabled.
I think the core_clk_rate variable should change when we change the lcd
clock source through dss_select_lcd_clk_source() for omap3.
If we have the following sequence:
...
dispc_mgr_set_lcd_divisor();
dss_select_lcd_clk_source();
...
The value of core_clock variable would be based on the previous clock
source, and not the current one.
This situation doesn't occur currently as the 'apply' framework delays
all dispc writes to the point when we enable the manager. So the
sequence above cannot occur. But maybe we should keep this in mind when
we move more things to omapdrm, where 'apply' isn't in use.
Archit
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 05ff2b9..f564955 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -97,6 +97,8 @@ static struct {
>
> int irq;
>
> + unsigned long core_clk_rate;
> +
> u32 fifo_size[DISPC_MAX_NR_FIFOS];
> /* maps which plane is using a fifo. fifo-id -> plane-id */
> int fifo_assignment[DISPC_MAX_NR_FIFOS];
> @@ -2951,6 +2953,10 @@ static void dispc_mgr_set_lcd_divisor(enum omap_channel channel, u16 lck_div,
>
> dispc_write_reg(DISPC_DIVISORo(channel),
> FLD_VAL(lck_div, 23, 16) | FLD_VAL(pck_div, 7, 0));
> +
> + if (dss_has_feature(FEAT_CORE_CLK_DIV) = false &&
> + channel = OMAP_DSS_CHANNEL_LCD)
> + dispc.core_clk_rate = dispc_fclk_rate() / lck_div;
> }
>
> static void dispc_mgr_get_lcd_divisor(enum omap_channel channel, int *lck_div,
> @@ -3056,15 +3062,7 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
>
> unsigned long dispc_core_clk_rate(void)
> {
> - int lcd;
> - unsigned long fclk = dispc_fclk_rate();
> -
> - if (dss_has_feature(FEAT_CORE_CLK_DIV))
> - lcd = REG_GET(DISPC_DIVISOR, 23, 16);
> - else
> - lcd = REG_GET(DISPC_DIVISORo(OMAP_DSS_CHANNEL_LCD), 23, 16);
> -
> - return fclk / lcd;
> + return dispc.core_clk_rate;
> }
>
> static unsigned long dispc_plane_pclk_rate(enum omap_plane plane)
> @@ -3451,6 +3449,8 @@ static void _omap_dispc_initial_config(void)
> l = FLD_MOD(l, 1, 0, 0);
> l = FLD_MOD(l, 1, 23, 16);
> dispc_write_reg(DISPC_DIVISOR, l);
> +
> + dispc.core_clk_rate = dispc_fclk_rate();
> }
>
> /* FUNCGATED */
>
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Fabio Porcedda @ 2013-03-20 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201303201020.14654.arnd@arndb.de>
On Wed, Mar 20, 2013 at 11:20 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 20 March 2013, Fabio Porcedda wrote:
>> I think we can check inside the deferred_probe_work_func()
>> if the dev->probe function pointer is equal to platform_drv_probe_fail().
>
> I think it's too late by then, because that would only warn if we try to probe
> it again, but when platform_driver_probe() does not succeed immediately, it
Maybe you mean "does succeed immediately" ?
> unregisters the driver again, so we never get there.
--
Fabio Porcedda
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Arnd Bergmann @ 2013-03-20 10:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHkwnC-3_dDM3JO8y3yeNFz7=fpP=MtZ9D-3cMH8rNF9C1NZBA@mail.gmail.com>
On Wednesday 20 March 2013, Fabio Porcedda wrote:
> I think we can check inside the deferred_probe_work_func()
> if the dev->probe function pointer is equal to platform_drv_probe_fail().
I think it's too late by then, because that would only warn if we try to probe
it again, but when platform_driver_probe() does not succeed immediately, it
unregisters the driver again, so we never get there.
Arnd
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Fabio Porcedda @ 2013-03-20 9:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201303191759.27762.arnd@arndb.de>
On Tue, Mar 19, 2013 at 6:59 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 19 March 2013, Fabio Porcedda wrote:
>> On Tue, Mar 19, 2013 at 5:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Tuesday 19 March 2013, Geert Uytterhoeven wrote:
>> >> Hmm, so we may have drivers that (now) work perfectly fine with
>> >> module_platform_driver_probe()/platform_driver_probe(), but will start
>> >> failing suddenly in the future?
>> >
>> > They will fail if someone changes the initialization order. That would
>> > already break drivers before deferred probing support (and was the reason
>> > we added feature in the first place), but now we can be much more liberal
>> > with the order in which drivers are initialized, except when they are
>> > using platform_driver_probe()
>> >
>> >> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in
>> >> platform_driver_probe() to catch these?
>> >
>> > Yes, very good idea.
>>
>> If it's fine, I'll send a patch for that.
>
> That would be cool, yes. I looked at it earlier (after sending my email above)
> and couldn't find an easy way to do it though, because platform_drv_probe
> does not know whether it is called from platform_driver_probe or not.
>
> Maybe using something other than platform_driver_register would work here.
>
> Arnd
I think we can check inside the deferred_probe_work_func()
if the dev->probe function pointer is equal to platform_drv_probe_fail().
Regards
--
Fabio Porcedda
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Arnd Bergmann @ 2013-03-19 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHkwnC9FL9W07=n6bWvcwiE058zcBZwqUwtRB-VVNpU0gv0mNw@mail.gmail.com>
On Tuesday 19 March 2013, Fabio Porcedda wrote:
> On Tue, Mar 19, 2013 at 5:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 19 March 2013, Geert Uytterhoeven wrote:
> >> Hmm, so we may have drivers that (now) work perfectly fine with
> >> module_platform_driver_probe()/platform_driver_probe(), but will start
> >> failing suddenly in the future?
> >
> > They will fail if someone changes the initialization order. That would
> > already break drivers before deferred probing support (and was the reason
> > we added feature in the first place), but now we can be much more liberal
> > with the order in which drivers are initialized, except when they are
> > using platform_driver_probe()
> >
> >> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in
> >> platform_driver_probe() to catch these?
> >
> > Yes, very good idea.
>
> If it's fine, I'll send a patch for that.
That would be cool, yes. I looked at it earlier (after sending my email above)
and couldn't find an easy way to do it though, because platform_drv_probe
does not know whether it is called from platform_driver_probe or not.
Maybe using something other than platform_driver_register would work here.
Arnd
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Fabio Porcedda @ 2013-03-19 17:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201303191648.31527.arnd@arndb.de>
On Tue, Mar 19, 2013 at 5:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 19 March 2013, Geert Uytterhoeven wrote:
>> Hmm, so we may have drivers that (now) work perfectly fine with
>> module_platform_driver_probe()/platform_driver_probe(), but will start
>> failing suddenly in the future?
>
> They will fail if someone changes the initialization order. That would
> already break drivers before deferred probing support (and was the reason
> we added feature in the first place), but now we can be much more liberal
> with the order in which drivers are initialized, except when they are
> using platform_driver_probe()
>
>> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in
>> platform_driver_probe() to catch these?
>
> Yes, very good idea.
>
> Arnd
If it's fine, I'll send a patch for that.
Regards
--
Fabio Porcedda
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Arnd Bergmann @ 2013-03-19 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdVS56HRDSvr7XCpVEjEWnGti+V=J_m4qQzEid=23ON_fQ@mail.gmail.com>
On Tuesday 19 March 2013, Geert Uytterhoeven wrote:
> Hmm, so we may have drivers that (now) work perfectly fine with
> module_platform_driver_probe()/platform_driver_probe(), but will start
> failing suddenly in the future?
They will fail if someone changes the initialization order. That would
already break drivers before deferred probing support (and was the reason
we added feature in the first place), but now we can be much more liberal
with the order in which drivers are initialized, except when they are
using platform_driver_probe()
> I guess we need a big fat WARN_ON(-EPROBE_DEFER) in
> platform_driver_probe() to catch these?
Yes, very good idea.
Arnd
^ permalink raw reply
* Re: [PATCH] ARM: OMAP: remove "config FB_OMAP_CONSISTENT_DMA_SIZE"
From: Tomi Valkeinen @ 2013-03-19 13:26 UTC (permalink / raw)
To: Paul Bolle
Cc: Florian Tobias Schandinat, linux-fbdev, linux-omap, linux-kernel
In-Reply-To: <1363687677.1390.6.camel@x61.thuisdomein>
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]
On 2013-03-19 12:07, Paul Bolle wrote:
> The only user of Kconfig symbol FB_OMAP_CONSISTENT_DMA_SIZE got removed
> in v3.8, with commit 6ba54ab4a49bbad736b0254aa6bdf0cb83013815 ("ARM:
> OMAP: Remove omap_init_consistent_dma_size()"). Remove this symbol too.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> Eyeball tested only.
>
> drivers/video/omap/Kconfig | 11 -----------
> 1 file changed, 11 deletions(-)
>
> diff --git a/drivers/video/omap/Kconfig b/drivers/video/omap/Kconfig
> index e512581..0bc3a93 100644
> --- a/drivers/video/omap/Kconfig
> +++ b/drivers/video/omap/Kconfig
> @@ -39,17 +39,6 @@ config FB_OMAP_LCD_MIPID
> the Mobile Industry Processor Interface DBI-C/DCS
> specification. (Supported LCDs: Philips LPH8923, Sharp LS041Y3)
>
> -config FB_OMAP_CONSISTENT_DMA_SIZE
> - int "Consistent DMA memory size (MB)"
> - depends on FB_OMAP
> - range 1 14
> - default 2
> - help
> - Increase the DMA consistent memory size according to your video
> - memory needs, for example if you want to use multiple planes.
> - The size must be 2MB aligned.
> - If unsure say 1.
> -
> config FB_OMAP_DMA_TUNE
> bool "Set DMA SDRAM access priority high"
> depends on FB_OMAP
>
Thanks, I'll add this to omapdss tree.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH v2] fbdev: sh_mobile_lcdc: fixup B side hsync adjust settings
From: Laurent Pinchart @ 2013-03-19 12:38 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <87txoqmo3p.wl%kuninori.morimoto.gx@renesas.com>
Hi Simon,
On Tuesday 19 March 2013 13:19:52 Simon Horman wrote:
> On Tue, Mar 05, 2013 at 04:45:46PM +0100, Laurent Pinchart wrote:
> > On Monday 04 March 2013 21:07:10 Kuninori Morimoto wrote:
> > > The lcdc B side horizon output is shifted
> > > if sh_mobile_lcdc_pan() was called.
> > > This patch fixup this issue.
> > > It is tested on R8A7740 Armadillo800eva HDMI output.
> > > Special thanks to Fukushima-san, and Sano-san
> > >
> > > Reported-by: Osamu Fukushima <osamu.fukushima.wr@renesas.com>
> > > Signed-off-by: Hideyuki Sano <hideyuki.sano.dn@renesas.com>
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Laurent, can you handle getting this merged?
> If not, please let me know who to prod.
We have no fbdev maintainer anymore. Can this be pushed through your tree ?
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: How to manage OMAP display drivers in the future
From: Tomi Valkeinen @ 2013-03-19 11:56 UTC (permalink / raw)
To: Rob Clark; +Cc: linux-fbdev, dri-devel@lists.freedesktop.org
In-Reply-To: <CAF6AEGs_6kLU=OR-mX9kvr=_t9-z4+StobksaW+MfcnhEu2XvA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]
On 2013-03-18 22:46, Rob Clark wrote:
> On Wed, Mar 13, 2013 at 4:57 AM, Tomi Valkeinen <tomba@iki.fi> wrote:
>> Hi Dave,
>>
>> I'm writing this mail to get some ideas how we should manage OMAP's
>> display drivers in the future.
>>
>> As a short intro, we have the following players around:
>>
>> omapdss - omapdss handles the DSS (display subsystem) hardware. omapdss
>> doesn't do any buffer management or expose any userspace API (except a
>> few sysfs files), so it doesn't do anything by itself.
>> (drivers/video/omap2/dss/)
>>
>> panel drivers - Drivers for various panel models. The panel drivers use
>> omapdss API to manage the video bus. (drivers/video/omap2/displays/)
>>
>> omapfb - Framebuffer driver, uses omapdss to handle the HW.
>> (drivers/video/omap2/omapfb/)
>>
>> omap_vout - V4L2 driver for showing video, uses omapdss to handle the
>> HW. (drivers/media/platform/omap/)
>>
>> omapdrm - DRM driver, uses omapdss to handle the HW.
>> (drivers/gpu/drm/omapdrm/)
>>
>> omapdss and the panel drivers form the lowest level layer. omapfb and
>> omap_vout can be used at the same time, but omapdrm must be used alone,
>> without omapfb or omap_vout.
>>
>> omapfb and omap_vout are not much developed anymore, even though they
>> are still commonly used. Most of the development happens in omapdss,
>> panel drivers and omapdrm.
>
> I'd guess if changes in omapfb or omap_vout are mainly just updates
> resulting from omapdss changes, maybe merging it all via drm tree
> would make most sense..
>
> Although I tend to think life would be easier w/ some copy/paste.. Ie.
> just move a copy of omapdss into omapdrm directory and start
> refactoring.. Offhand I think at least in the hdmi code, I think we
> could jettison the big timings table, and avi-infoframe stuff and use
> drm helpers. Probably other places where we could get rid of code
> that duplicates stuff that drm does (or could) provide helpers for..
I've been playing with that idea, but copying omapdss is not so
straightforward either. The panel drivers, headers, kconfig options,
board file code related to dss... It could easily create a rather
confusing mess.
I'm hoping that CDF in some form would realize soon, and then copying
omapdss would be at least somewhwat easier, as there's no need to drag
the panel drivers along.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0
From: Marek Vasut @ 2013-03-19 10:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130319054846.GI12462@S2101-09.ap.freescale.net>
Dear Shawn Guo,
> On Mon, Mar 18, 2013 at 04:19:52PM +0100, Marek Vasut wrote:
> > Dear Shawn Guo,
> >
> > > On Mon, Mar 18, 2013 at 03:18:13PM +0100, Marek Vasut wrote:
> > > > Did you manage to test it please?
> > >
> > > I do not have X11 environment handy to test, but framework console
> > > seems still working.
> >
> > Even after me poking in it, lol :-)
> >
> > You can grab [1], extract it onto card or NFS and just use it as root
> > filesystem, the X11 will come up automagically.
>
> Thanks, Marek. Yeah, now I can see the problem and your patch does fix
> it.
Whew, cool. Can you please apply it and push mainline so I can re-send the one
for -stable then?
Thank you!
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH] ARM: OMAP: remove "config FB_OMAP_CONSISTENT_DMA_SIZE"
From: Paul Bolle @ 2013-03-19 10:07 UTC (permalink / raw)
To: Tomi Valkeinen, Florian Tobias Schandinat
Cc: linux-fbdev, linux-omap, linux-kernel
The only user of Kconfig symbol FB_OMAP_CONSISTENT_DMA_SIZE got removed
in v3.8, with commit 6ba54ab4a49bbad736b0254aa6bdf0cb83013815 ("ARM:
OMAP: Remove omap_init_consistent_dma_size()"). Remove this symbol too.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Eyeball tested only.
drivers/video/omap/Kconfig | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/video/omap/Kconfig b/drivers/video/omap/Kconfig
index e512581..0bc3a93 100644
--- a/drivers/video/omap/Kconfig
+++ b/drivers/video/omap/Kconfig
@@ -39,17 +39,6 @@ config FB_OMAP_LCD_MIPID
the Mobile Industry Processor Interface DBI-C/DCS
specification. (Supported LCDs: Philips LPH8923, Sharp LS041Y3)
-config FB_OMAP_CONSISTENT_DMA_SIZE
- int "Consistent DMA memory size (MB)"
- depends on FB_OMAP
- range 1 14
- default 2
- help
- Increase the DMA consistent memory size according to your video
- memory needs, for example if you want to use multiple planes.
- The size must be 2MB aligned.
- If unsure say 1.
-
config FB_OMAP_DMA_TUNE
bool "Set DMA SDRAM access priority high"
depends on FB_OMAP
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Geert Uytterhoeven @ 2013-03-19 9:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHkwnC9fg7_uhLM2KD3vvj_oFx3EBoQfw8mCN=V9pyV5=k37aA@mail.gmail.com>
On Tue, Mar 19, 2013 at 9:55 AM, Fabio Porcedda
<fabio.porcedda@gmail.com> wrote:
> On Mon, Mar 18, 2013 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Monday 18 March 2013, Fabio Porcedda wrote:
>>> On Mon, Mar 18, 2013 at 11:58 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> > On Monday 18 March 2013, Fabio Porcedda wrote:
>>> >> Since by using platform_driver_probe() the function
>>> >> ep93xx_pwm_probe() is freed after initialization,
>>> >> is better to use module_platform_drive_probe().
>>> >> IMHO i don't see any good reason to use module_platform_driver() for
>>> >> this driver.
>>> >
>>> > As I commented earlier, the platform_driver_probe() and
>>> > module_platform_drive_probe() interfaces are rather dangerous in combination
>>> > with deferred probing, I would much prefer Harley's patch.
>>>
>>> Since those drivers don't use -EPROBE_DEFER i was thinking that they don't use
>>> deferred probing.
>>> I'm missing something?
>>
>> clk_get() may return -EPROBE_DEFER after ep93xx is converted to use the
>> common clk API. We currently return the value of clk_get from the probe()
>> function, which will automatically do the right thing as long as the probe
>> function remains reachable.
>
> Thanks for the explanation.
Hmm, so we may have drivers that (now) work perfectly fine with
module_platform_driver_probe()/platform_driver_probe(), but will start
failing suddenly in the future?
I guess we need a big fat WARN_ON(-EPROBE_DEFER) in
platform_driver_probe() to catch these?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 10/10] drivers: misc: use module_platform_driver_probe()
From: Fabio Porcedda @ 2013-03-19 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201303181128.45215.arnd@arndb.de>
On Mon, Mar 18, 2013 at 12:28 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 18 March 2013, Fabio Porcedda wrote:
>>
>> On Mon, Mar 18, 2013 at 11:58 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday 18 March 2013, Fabio Porcedda wrote:
>> >> Since by using platform_driver_probe() the function
>> >> ep93xx_pwm_probe() is freed after initialization,
>> >> is better to use module_platform_drive_probe().
>> >> IMHO i don't see any good reason to use module_platform_driver() for
>> >> this driver.
>> >
>> > As I commented earlier, the platform_driver_probe() and
>> > module_platform_drive_probe() interfaces are rather dangerous in combination
>> > with deferred probing, I would much prefer Harley's patch.
>>
>> Since those drivers don't use -EPROBE_DEFER i was thinking that they don't use
>> deferred probing.
>> I'm missing something?
>
> clk_get() may return -EPROBE_DEFER after ep93xx is converted to use the
> common clk API. We currently return the value of clk_get from the probe()
> function, which will automatically do the right thing as long as the probe
> function remains reachable.
Thanks for the explanation.
Regards
Fabio Porcedda
> Arnd
^ permalink raw reply
* Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0
From: Shawn Guo @ 2013-03-19 5:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1363631042-5321-1-git-send-email-marex@denx.de>
On Mon, Mar 18, 2013 at 07:24:02PM +0100, Marek Vasut wrote:
> The issue fixed by this patch manifests only then using X11
> with mxsfb driver. The X11 will display either shifted image
> or otherwise distorted image on the LCD.
>
> The problem is that the X11 tries to reconfigure the framebuffer
> and along the way calls fb_ops.fb_set_par() with X11's desired
> configuration values. The field of particular interest is
> fb_info->var.sync which contains non-standard values if
> configured by kernel. These are either FB_SYNC_DATA_ENABLE_HIGH_ACT,
> FB_SYNC_DOTCLK_FAILING_ACT or both, depending on the platform
> configuration. Both of these values are defined in the
> include/linux/mxsfb.h file.
>
> The driver interprets these values and configures the LCD controller
> accordingly. Yet X11 only has access to the standard values for this
> field defined in include/uapi/linux/fb.h and thus, unlike kernel,
> omits these special values. This results in distorted image on the
> LCD.
>
> This patch moves these non-standard values into new field of the
> mxsfb_platform_data structure so the driver can in turn check this
> field instead of the video mode field for these specific portions.
>
> Moreover, this patch prefixes these values with MXSFB_SYNC_ prefix
> instead of FB_SYNC_ prefix to prevent confusion of subsequent users.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Lothar Waßmann <LW@karo-electronics.de>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
Applied with one trivial change below.
...
> @@ -407,6 +404,8 @@ static void __init cfa10049_init(void)
> mxsfb_pdata.mode_count = ARRAY_SIZE(cfa10049_video_modes);
> mxsfb_pdata.default_bpp = 32;
> mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> + mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
> +
I removed this unnecessary new line.
Shawn
> }
^ permalink raw reply
* Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0
From: Shawn Guo @ 2013-03-19 5:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201303181619.52377.marex@denx.de>
On Mon, Mar 18, 2013 at 04:19:52PM +0100, Marek Vasut wrote:
> Dear Shawn Guo,
>
> > On Mon, Mar 18, 2013 at 03:18:13PM +0100, Marek Vasut wrote:
> > > Did you manage to test it please?
> >
> > I do not have X11 environment handy to test, but framework console
> > seems still working.
>
> Even after me poking in it, lol :-)
>
> You can grab [1], extract it onto card or NFS and just use it as root
> filesystem, the X11 will come up automagically.
>
Thanks, Marek. Yeah, now I can see the problem and your patch does fix
it.
Shawn
> [1] ftp://ftp.denx.de/pub/eldk/5.3/targets/armv5te/core-image-sato-sdk-generic-
> armv5te.tar.gz
^ permalink raw reply
* Re: [PATCH v2] fbdev: sh_mobile_lcdc: fixup B side hsync adjust settings
From: Simon Horman @ 2013-03-19 4:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <87txoqmo3p.wl%kuninori.morimoto.gx@renesas.com>
On Tue, Mar 05, 2013 at 04:45:46PM +0100, Laurent Pinchart wrote:
> Hi Morimoto-san,
>
> Thanks for the patch.
>
> On Monday 04 March 2013 21:07:10 Kuninori Morimoto wrote:
> > The lcdc B side horizon output is shifted
> > if sh_mobile_lcdc_pan() was called.
> > This patch fixup this issue.
> > It is tested on R8A7740 Armadillo800eva HDMI output.
> > Special thanks to Fukushima-san, and Sano-san
> >
> > Reported-by: Osamu Fukushima <osamu.fukushima.wr@renesas.com>
> > Signed-off-by: Hideyuki Sano <hideyuki.sano.dn@renesas.com>
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Laurent, can you handle getting this merged?
If not, please let me know who to prod.
>
> > ---
> > v1 -> v2
> >
> > - git log has tested SoC/board
> >
> > drivers/video/sh_mobile_lcdcfb.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index 63203ac..0264704 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > @@ -858,6 +858,7 @@ static void sh_mobile_lcdc_geometry(struct
> > sh_mobile_lcdc_chan *ch) tmp = ((mode->xres & 7) << 24) | ((display_h_total
> > & 7) << 16)
> >
> > | ((mode->hsync_len & 7) << 8) | (hsync_pos & 7);
> >
> > lcdc_write_chan(ch, LDHAJR, tmp);
> > + lcdc_write_chan_mirror(ch, LDHAJR, tmp);
> > }
> >
> > static void sh_mobile_lcdc_overlay_setup(struct sh_mobile_lcdc_overlay
> > *ovl)
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0
From: Marek Vasut @ 2013-03-18 22:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130318220134.GC5062@elie.Belkin>
Hi Jonathan,
> Marek Vasut wrote:
> > - It or an equivalent fix must already exist in Linus' tree (upstream).
> >
> > This can not happen, the fix in Linus' tree will have to be different
> > since in v3.9 there is one more MX28 platform which also needs fixing. I
> > suppose I will now wait for Shawn to merge the patch for 3.9-rc, get it
> > mainline and then this one can be merged?
>
> Yes, please write again with a patch referencing the upstream commit
> id when this has been fixed upstream.
Roger that, will do. Is that all that needs repairing in the patch?
Thank you!
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0
From: Jonathan Nieder @ 2013-03-18 22:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201303182231.22749.marex@denx.de>
Marek Vasut wrote:
> - It or an equivalent fix must already exist in Linus' tree (upstream).
> This can not happen, the fix in Linus' tree will have to be different since in
> v3.9 there is one more MX28 platform which also needs fixing. I suppose I will
> now wait for Shawn to merge the patch for 3.9-rc, get it mainline and then this
> one can be merged?
Yes, please write again with a patch referencing the upstream commit
id when this has been fixed upstream.
Thanks and hope that helps,
Jonathan
^ permalink raw reply
* Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0
From: Marek Vasut @ 2013-03-18 21:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130318185810.GA11040@kroah.com>
Hi Greg,
[...]
> > NOTE: This is the version for stable v3.8.3, so I'm sending it to
> > -stable.
> >
> > I will send adjusted version for mainline 3.9-rc , since there is
> > one more board in mainline and therefore the versions of the patch
> > must differ.
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
I'm somewhat sure I violated a few (see below), I won't argue there as you
surely are much more experienced, but let me dissect the rules so I can learn
which one to be careful about. Please help me if I did not understand something
properly.
- It must be obviously correct and tested.
YES, Fabio tested it on MX28EVK, me on M28EVK (two different devices)
- It cannot be bigger than 100 lines, with context.
NO, but I see no way to compress it under 100 lines.
- It must fix only one thing.
YES, it fixes broken X11 on MX28
- It must fix a real bug that bothers people (not a, "This could be a
problem..." type thing).
YES, it fixes broken X11 on MX28
- It must fix a problem that causes a build error (but not for things
marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
security issue, or some "oh, that's not good" issue. In short, something
critical.
YES, it fixes broken X11 on MX28
- Serious issues as reported by a user of a distribution kernel may also
be considered if they fix a notable performance or interactivity issue.
As these fixes are not as obvious and have a higher risk of a subtle
regression they should only be submitted by a distribution kernel
maintainer and include an addendum linking to a bugzilla entry if it
exists and additional information on the user-visible impact.
This doesn't apply I guess?
- New device IDs and quirks are also accepted.
This doesn't apply for sure.
- No "theoretical race condition" issues, unless an explanation of how the
race can be exploited is also provided.
This doesn't apply for sure.
- It cannot contain any "trivial" fixes in it (spelling changes,
whitespace cleanups, etc).
This doesn't apply for sure.
- It must follow the Documentation/SubmittingPatches rules.
I believe it does. It has one checkpatch warning, but to keep the amount of
changes to bare minimum, I left it in there.
- It or an equivalent fix must already exist in Linus' tree (upstream).
This can not happen, the fix in Linus' tree will have to be different since in
v3.9 there is one more MX28 platform which also needs fixing. I suppose I will
now wait for Shawn to merge the patch for 3.9-rc, get it mainline and then this
one can be merged?
Thank you!
Best regards,
Marek Vasut
^ permalink raw reply
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