* Re: [PATCH 24/25] OMAPDSS: DSI: improve DSI module id handling
From: Archit Taneja @ 2012-05-04 9:21 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336053481-25433-25-git-send-email-tomi.valkeinen@ti.com>
On Thursday 03 May 2012 07:28 PM, Tomi Valkeinen wrote:
> We currently use the id of the dsi platform device (dsidev->id) as the
> DSI hardware module ID. This works because we assign the ID manually in
> arch/arm/mach-omap2/display.c at boot time.
>
> However, with device tree the platform device IDs are automatically
> assigned to an arbitrary number, and we can't use it.
If this number is arbitrary we would need to change the "dsi_pdev_map"
approach of mapping a dsi module and it's corresponding platform device.
Currently dsi_pdev_map is:
static struct platform_device *dsi_pdev_map[MAX_NUM_DSI];
So we either need to increase the array size to take larger arbitrary
numbers, or do something else.
We would also need to fix the usage of dsi_get_dsidev_from_id(), as
right now we manually pass 0 and 1 to it only, for example:
static void dsi1_dump_irqs(struct seq_file *s)
{
struct platform_device *dsidev = dsi_get_dsidev_from_id(0);
dsi_dump_dsidev_irqs(dsidev, s);
}
The immediate solution that comes to mind is to maintain 2 id's, one
which is sequential, and the other which the DT has created, and keep an
array to map these. But this seems messy!
Archit
>
> Instead of using dsidev->id during operation, this patch stores the
> value of dsidev->id to a private field of the dsi driver at probe(). The
> future device tree code can thus set the private field with some other
> way.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dsi.c | 46 +++++++++++++++++++----------------------
> 1 file changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 6cc92a8..ce964dd 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -256,6 +256,8 @@ struct dsi_data {
> struct platform_device *pdev;
> void __iomem *base;
>
> + int module_id;
> +
> int irq;
>
> struct clk *dss_clk;
> @@ -358,11 +360,6 @@ struct platform_device *dsi_get_dsidev_from_id(int module)
> return dsi_pdev_map[module];
> }
>
> -static inline int dsi_get_dsidev_id(struct platform_device *dsidev)
> -{
> - return dsidev->id;
> -}
> -
> static inline void dsi_write_reg(struct platform_device *dsidev,
> const struct dsi_reg idx, u32 val)
> {
> @@ -1181,10 +1178,9 @@ static unsigned long dsi_get_txbyteclkhs(struct platform_device *dsidev)
> static unsigned long dsi_fclk_rate(struct platform_device *dsidev)
> {
> unsigned long r;
> - int dsi_module = dsi_get_dsidev_id(dsidev);
> struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
>
> - if (dss_get_dsi_clk_source(dsi_module) = OMAP_DSS_CLK_SRC_FCK) {
> + if (dss_get_dsi_clk_source(dsi->module_id) = OMAP_DSS_CLK_SRC_FCK) {
> /* DSI FCLK source is DSS_CLK_FCK */
> r = clk_get_rate(dsi->dss_clk);
> } else {
> @@ -1683,7 +1679,7 @@ static void dsi_dump_dsidev_clocks(struct platform_device *dsidev,
> struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> struct dsi_clock_info *cinfo =&dsi->current_cinfo;
> enum omap_dss_clk_source dispc_clk_src, dsi_clk_src;
> - int dsi_module = dsi_get_dsidev_id(dsidev);
> + int dsi_module = dsi->module_id;
>
> dispc_clk_src = dss_get_dispc_clk_source();
> dsi_clk_src = dss_get_dsi_clk_source(dsi_module);
> @@ -1755,7 +1751,6 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev,
> struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> unsigned long flags;
> struct dsi_irq_stats stats;
> - int dsi_module = dsi_get_dsidev_id(dsidev);
>
> spin_lock_irqsave(&dsi->irq_stats_lock, flags);
>
> @@ -1772,7 +1767,7 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev,
> #define PIS(x) \
> seq_printf(s, "%-20s %10d\n", #x, stats.dsi_irqs[ffs(DSI_IRQ_##x)-1]);
>
> - seq_printf(s, "-- DSI%d interrupts --\n", dsi_module + 1);
> + seq_printf(s, "-- DSI%d interrupts --\n", dsi->module_id + 1);
> PIS(VC0);
> PIS(VC1);
> PIS(VC2);
> @@ -2272,7 +2267,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
>
> DSSDBGF();
>
> - r = dss_dsi_enable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> + r = dss_dsi_enable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
> if (r)
> return r;
>
> @@ -2382,20 +2377,21 @@ err_cio_pwr:
> dsi_cio_disable_lane_override(dsidev);
> err_scp_clk_dom:
> dsi_disable_scp_clk(dsidev);
> - dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> + dss_dsi_disable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
> return r;
> }
>
> static void dsi_cio_uninit(struct omap_dss_device *dssdev)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
>
> /* DDR_CLK_ALWAYS_ON */
> REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 0, 13, 13);
>
> dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_OFF);
> dsi_disable_scp_clk(dsidev);
> - dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> + dss_dsi_disable_pads(dsi->module_id, dsi_get_lane_mask(dssdev));
> }
>
> static void dsi_config_tx_fifo(struct platform_device *dsidev,
> @@ -4277,7 +4273,7 @@ static int dsi_configure_dispc_clocks(struct omap_dss_device *dssdev)
> static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> - int dsi_module = dsi_get_dsidev_id(dsidev);
> + struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> int r;
>
> r = dsi_pll_init(dsidev, true, true);
> @@ -4289,7 +4285,7 @@ static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
> goto err1;
>
> dss_select_dispc_clk_source(dssdev->clocks.dispc.dispc_fclk_src);
> - dss_select_dsi_clk_source(dsi_module, dssdev->clocks.dsi.dsi_fclk_src);
> + dss_select_dsi_clk_source(dsi->module_id, dssdev->clocks.dsi.dsi_fclk_src);
> dss_select_lcd_clk_source(dssdev->manager->id,
> dssdev->clocks.dispc.channel.lcd_clk_src);
>
> @@ -4328,7 +4324,7 @@ err3:
> dsi_cio_uninit(dssdev);
> err2:
> dss_select_dispc_clk_source(OMAP_DSS_CLK_SRC_FCK);
> - dss_select_dsi_clk_source(dsi_module, OMAP_DSS_CLK_SRC_FCK);
> + dss_select_dsi_clk_source(dsi->module_id, OMAP_DSS_CLK_SRC_FCK);
> dss_select_lcd_clk_source(dssdev->manager->id, OMAP_DSS_CLK_SRC_FCK);
>
> err1:
> @@ -4342,7 +4338,6 @@ static void dsi_display_uninit_dsi(struct omap_dss_device *dssdev,
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
> - int dsi_module = dsi_get_dsidev_id(dsidev);
>
> if (enter_ulps&& !dsi->ulps_enabled)
> dsi_enter_ulps(dsidev);
> @@ -4355,7 +4350,7 @@ static void dsi_display_uninit_dsi(struct omap_dss_device *dssdev,
> dsi_vc_enable(dsidev, 3, 0);
>
> dss_select_dispc_clk_source(OMAP_DSS_CLK_SRC_FCK);
> - dss_select_dsi_clk_source(dsi_module, OMAP_DSS_CLK_SRC_FCK);
> + dss_select_dsi_clk_source(dsi->module_id, OMAP_DSS_CLK_SRC_FCK);
> dss_select_lcd_clk_source(dssdev->manager->id, OMAP_DSS_CLK_SRC_FCK);
> dsi_cio_uninit(dssdev);
> dsi_pll_uninit(dsidev, disconnect_lanes);
> @@ -4616,7 +4611,7 @@ static void dsi_put_clocks(struct platform_device *dsidev)
> static int __init omap_dsihw_probe(struct platform_device *dsidev)
> {
> u32 rev;
> - int r, i, dsi_module = dsi_get_dsidev_id(dsidev);
> + int r, i;
> struct resource *dsi_mem;
> struct dsi_data *dsi;
> struct omap_dss_board_info *pdata = dsidev->dev.platform_data;
> @@ -4625,8 +4620,9 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev)
> if (!dsi)
> return -ENOMEM;
>
> + dsi->module_id = dsidev->id;
> dsi->pdev = dsidev;
> - dsi_pdev_map[dsi_module] = dsidev;
> + dsi_pdev_map[dsi->module_id] = dsidev;
> dev_set_drvdata(&dsidev->dev, dsi);
>
> spin_lock_init(&dsi->irq_lock);
> @@ -4712,7 +4708,7 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev)
> if (dssdev->type != OMAP_DISPLAY_TYPE_DSI)
> continue;
>
> - if (dssdev->phy.dsi.module != dsi_module)
> + if (dssdev->phy.dsi.module != dsi->module_id)
> continue;
>
> r = dsi_init_display(dssdev);
> @@ -4729,15 +4725,15 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev)
>
> dsi_runtime_put(dsidev);
>
> - if (dsi_module = 0)
> + if (dsi->module_id = 0)
> dss_debugfs_create_file("dsi1_regs", dsi1_dump_regs);
> - else if (dsi_module = 1)
> + else if (dsi->module_id = 1)
> dss_debugfs_create_file("dsi2_regs", dsi2_dump_regs);
>
> #ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
> - if (dsi_module = 0)
> + if (dsi->module_id = 0)
> dss_debugfs_create_file("dsi1_irqs", dsi1_dump_irqs);
> - else if (dsi_module = 1)
> + else if (dsi->module_id = 1)
> dss_debugfs_create_file("dsi2_irqs", dsi2_dump_irqs);
> #endif
> return 0;
^ permalink raw reply
* Re: [PATCH 11/25] OMAPDSS: create custom pdevs for DSS omap_devices
From: Tomi Valkeinen @ 2012-05-04 9:00 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA39091.8010007@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2912 bytes --]
On Fri, 2012-05-04 at 13:47 +0530, Archit Taneja wrote:
> On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
> > @@ -221,22 +279,24 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> > oh_count = ARRAY_SIZE(omap4_dss_hwmod_data);
> > }
> >
> > - for (i = 0; i< oh_count; i++) {
> > - oh = omap_hwmod_lookup(curr_dss_hwmod[i].oh_name);
> > - if (!oh) {
> > - pr_err("Could not look up %s\n",
> > - curr_dss_hwmod[i].oh_name);
> > - return -ENODEV;
> > - }
> > + dss_pdev = NULL;
> >
> > - pdev = omap_device_build(curr_dss_hwmod[i].dev_name,
> > - curr_dss_hwmod[i].id, oh,
> > + for (i = 0; i< oh_count; i++) {
> > + pdev = create_dss_pdev(curr_dss_hwmod[i].dev_name,
> > + curr_dss_hwmod[i].id,
> > + curr_dss_hwmod[i].oh_name,
> > NULL, 0,
> > - NULL, 0, 0);
> > + dss_pdev);
> > +
> > + if (IS_ERR(pdev)) {
> > + pr_err("Could not build omap_device for %s\n",
> > + curr_dss_hwmod[i].oh_name);
> > +
> > + return PTR_ERR(pdev);
> > + }
> >
> > - if (WARN((IS_ERR(pdev)), "Could not build omap_device for %s\n",
> > - curr_dss_hwmod[i].oh_name))
> > - return -ENODEV;
> > + if (i == 0)
> > + dss_pdev = pdev;
>
> The above line is a bit tricky to understand, maybe something like this
> may explain the parent-child setting better:
>
> if (!strcmp(curr_dss_hwmod[i].oh_name, "dss_core"))
> dss_pdev = pdev;
I agree that it's a bit confusing. But your suggestion is not very good
either, as the code does not work properly if the dss_core is not the
first one created. I'll look into it. Perhaps I can separate the code
into a small function, and then I can more easily do something like:
dss_pdev = create_the_device();
for () {
// create the rest of the devices
create_the_device();
}
and that would clarify what's going on.
> I had another general question about the parent-child series. What is
> the use of the platform device omap_display_device (with the name
> "omapdss"). Is it just a way to get the board data?
Originally, before hwmods, we had only omapdss device, which contained
all the dss code. Then came hwmods, and the omapdss was split into
smaller devices, but omapdss was still there.
As I see it, omapdss is currently a "virtual" higher level device
(virtual in the sense that it doesn't correspond directly to any HW),
and the code for omapdss is more or less in the core.c file. It's used
to pass the board data, but also has some generic dss stuff that all dss
subdevices can use.
I think in the long run we should remove omapdss device, and probably
handle the generic stuff in dss_core (dss.c), which, as a parent to
other subdevices, should fit fine for the role.
For the time being we can't remove it. It's the only simple way to pass
callbacks from the arch code with device tree.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 08/25] OMAPDSS: clean up the omapdss platform data mess
From: Tomi Valkeinen @ 2012-05-04 8:49 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA39501.3070205@ti.com>
[-- Attachment #1: Type: text/plain, Size: 850 bytes --]
On Fri, 2012-05-04 at 14:06 +0530, Archit Taneja wrote:
> On Friday 04 May 2012 02:02 PM, Tomi Valkeinen wrote:
> > Hmm. The sequence shouldn't change. The order in which the devices are
> > registered doesn't matter if there are no drivers registered yet. When
> > the drivers are registered, and there's a device for it, the probe will
> > be done. So in this case the order of the driver registration will
> > dictate the order of probes.
>
> Oh okay, I don't know where the initcalls exactly happen, but I guess
> they will happen after the .init_machine op in the board file.
>
> Do you know where the initcalls happen, I couldn't find it by browsing
> the kernel code :)
Well, include/linux/init.h lists the inits in order. machine init is not
there, I guess it's not part of the init order, but even earlier.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 08/25] OMAPDSS: clean up the omapdss platform data mess
From: Archit Taneja @ 2012-05-04 8:48 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336120320.2701.1.camel@deskari>
On Friday 04 May 2012 02:02 PM, Tomi Valkeinen wrote:
> On Fri, 2012-05-04 at 11:02 +0530, Archit Taneja wrote:
>> Hi,
>>
>> On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
>>> The omapdss pdata handling is a mess. This is more evident when trying
>>> to use device tree for DSS, as we don't have platform data anymore in
>>> that case. This patch cleans the pdata handling by:
>>>
>>> - Remove struct omap_display_platform_data. It was used just as a
>>> wrapper for struct omap_dss_board_info.
>>> - Pass the platform data only to omapdss device. The drivers for omap
>>> dss hwmods do not need the platform data. This should also work better
>>> for DT, as we can create omapdss device programmatically in generic omap
>>> boot code, and thus we can pass the pdata to it.
>>> - Create dss functions for get_ctx_loss_count and dsi_enable/disable_pads
>>> that the dss hwmod drivers can call.
>>>
>>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
>>> ---
>>> arch/arm/mach-omap2/display.c | 39 +++++++++++++++++++--------------------
>>> drivers/video/omap2/dss/core.c | 35 +++++++++++++++++++++++++++++++++++
>>> drivers/video/omap2/dss/dispc.c | 21 ++-------------------
>>> drivers/video/omap2/dss/dsi.c | 17 +++--------------
>>> drivers/video/omap2/dss/dss.h | 3 +++
>>> drivers/video/omap2/dss/hdmi.c | 2 --
>>> include/video/omapdss.h | 5 -----
>>> 7 files changed, 62 insertions(+), 60 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>>> index 60cded4..07232fd 100644
>>> --- a/arch/arm/mach-omap2/display.c
>>> +++ b/arch/arm/mach-omap2/display.c
>>> @@ -191,10 +191,24 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
>>> struct omap_hwmod *oh;
>>> struct platform_device *pdev;
>>> int i, oh_count;
>>> - struct omap_display_platform_data pdata;
>>> const struct omap_dss_hwmod_data *curr_dss_hwmod;
>>>
>>> - memset(&pdata, 0, sizeof(pdata));
>>> + /* create omapdss device */
>>> +
>>> + board_data->dsi_enable_pads = omap_dsi_enable_pads;
>>> + board_data->dsi_disable_pads = omap_dsi_disable_pads;
>>> + board_data->get_context_loss_count = omap_pm_get_dev_context_loss_count;
>>> + board_data->set_min_bus_tput = omap_dss_set_min_bus_tput;
>>> +
>>> + omap_display_device.dev.platform_data = board_data;
>>> +
>>> + r = platform_device_register(&omap_display_device);
>>> + if (r< 0) {
>>> + pr_err("Unable to register omapdss device\n");
>>> + return r;
>>> + }
>>
>> After this patch, the "omapdss" platform device is registered before the
>> other dss platform devices. This would change the sequence of probes of
>> these devices. Was this intentional?
>
> Hmm. The sequence shouldn't change. The order in which the devices are
> registered doesn't matter if there are no drivers registered yet. When
> the drivers are registered, and there's a device for it, the probe will
> be done. So in this case the order of the driver registration will
> dictate the order of probes.
Oh okay, I don't know where the initcalls exactly happen, but I guess
they will happen after the .init_machine op in the board file.
Do you know where the initcalls happen, I couldn't find it by browsing
the kernel code :)
Archit
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
From: Jean Delvare @ 2012-05-04 8:39 UTC (permalink / raw)
To: Wolfram Sang
Cc: Mark Brown, Florian Tobias Schandinat, Dmitry Torokhov, linux-i2c,
linux-input, linux-fbdev
In-Reply-To: <20120503113546.GG9574@pengutronix.de>
Hi Wolfram,
On Thu, 3 May 2012 13:35:46 +0200, Wolfram Sang wrote:
> On Thu, May 03, 2012 at 11:53:36AM +0100, Mark Brown wrote:
> > Since there are uses for I2C_M_NOSTART which are much more sensible and
> > standard than most of the protocol mangling functionality (the main one
> > being gather writes to devices where something like a register address
> > needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> > for this feature and update all the users to use it.
> >
> > In the case of regmap-i2c we remove the requirement for mangling as
> > I2C_M_NOSTART is the only mangling feature which is being used.
> >
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> Applied to next, thanks! Jean, let me know if you prefer to take it.
> Acks from input and fbdev maintainers still appreciated.
I'd prefer to take it, yes, as it touches the core infrastructure.
Thanks,
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH 11/25] OMAPDSS: create custom pdevs for DSS omap_devices
From: Tomi Valkeinen @ 2012-05-04 8:37 UTC (permalink / raw)
To: Archit Taneja, Paul Walmsley, Kevin Hilman; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA37139.9020501@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]
On Fri, 2012-05-04 at 11:33 +0530, Archit Taneja wrote:
> Hi,
>
> On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
> > Instead of using omap_device_build() to create the omap_devices for DSS
> > hwmods, create them with a custom function. This will allow us to create
> > a parent-child hierarchy for the devices so that the omapdss_core device
> > is parent for the rest of the dss hwmod devices.
> >
> > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> > ---
> > arch/arm/mach-omap2/display.c | 88 ++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 74 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> > index 07232fd..46d2a98 100644
> > --- a/arch/arm/mach-omap2/display.c
> > +++ b/arch/arm/mach-omap2/display.c
> > @@ -185,13 +185,71 @@ static int omap_dss_set_min_bus_tput(struct device *dev, unsigned long tput)
> > return omap_pm_set_min_bus_tput(dev, OCP_INITIATOR_AGENT, tput);
> > }
> >
> > +static struct platform_device *create_dss_pdev(const char *pdev_name,
> > + int pdev_id, const char *oh_name, void *pdata, int pdata_len,
> > + struct platform_device *parent)
>
> This function looks quite generic, it's like omap_device_build() but
> with a parent associated. omap_device_build() seems to be a special case
> of this function with parent passed as null. Won't this
> function be needed by other devices too?
>
> Maybe we could modify omap_device_build_ss() to take a parent argument,
> and make a function called omap_device_build_parent(), where both
> omap_device_build() and omap_device_build_parent() call
> omap_device_build_ss()?
Yes, that sounds good to me.
Paul, Kevin, what do you think, could the omap_device functions be
extended to allow setting a parent device?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 08/25] OMAPDSS: clean up the omapdss platform data mess
From: Tomi Valkeinen @ 2012-05-04 8:32 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4FA369D2.1070809@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]
On Fri, 2012-05-04 at 11:02 +0530, Archit Taneja wrote:
> Hi,
>
> On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
> > The omapdss pdata handling is a mess. This is more evident when trying
> > to use device tree for DSS, as we don't have platform data anymore in
> > that case. This patch cleans the pdata handling by:
> >
> > - Remove struct omap_display_platform_data. It was used just as a
> > wrapper for struct omap_dss_board_info.
> > - Pass the platform data only to omapdss device. The drivers for omap
> > dss hwmods do not need the platform data. This should also work better
> > for DT, as we can create omapdss device programmatically in generic omap
> > boot code, and thus we can pass the pdata to it.
> > - Create dss functions for get_ctx_loss_count and dsi_enable/disable_pads
> > that the dss hwmod drivers can call.
> >
> > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> > ---
> > arch/arm/mach-omap2/display.c | 39 +++++++++++++++++++--------------------
> > drivers/video/omap2/dss/core.c | 35 +++++++++++++++++++++++++++++++++++
> > drivers/video/omap2/dss/dispc.c | 21 ++-------------------
> > drivers/video/omap2/dss/dsi.c | 17 +++--------------
> > drivers/video/omap2/dss/dss.h | 3 +++
> > drivers/video/omap2/dss/hdmi.c | 2 --
> > include/video/omapdss.h | 5 -----
> > 7 files changed, 62 insertions(+), 60 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> > index 60cded4..07232fd 100644
> > --- a/arch/arm/mach-omap2/display.c
> > +++ b/arch/arm/mach-omap2/display.c
> > @@ -191,10 +191,24 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> > struct omap_hwmod *oh;
> > struct platform_device *pdev;
> > int i, oh_count;
> > - struct omap_display_platform_data pdata;
> > const struct omap_dss_hwmod_data *curr_dss_hwmod;
> >
> > - memset(&pdata, 0, sizeof(pdata));
> > + /* create omapdss device */
> > +
> > + board_data->dsi_enable_pads = omap_dsi_enable_pads;
> > + board_data->dsi_disable_pads = omap_dsi_disable_pads;
> > + board_data->get_context_loss_count = omap_pm_get_dev_context_loss_count;
> > + board_data->set_min_bus_tput = omap_dss_set_min_bus_tput;
> > +
> > + omap_display_device.dev.platform_data = board_data;
> > +
> > + r = platform_device_register(&omap_display_device);
> > + if (r< 0) {
> > + pr_err("Unable to register omapdss device\n");
> > + return r;
> > + }
>
> After this patch, the "omapdss" platform device is registered before the
> other dss platform devices. This would change the sequence of probes of
> these devices. Was this intentional?
Hmm. The sequence shouldn't change. The order in which the devices are
registered doesn't matter if there are no drivers registered yet. When
the drivers are registered, and there's a device for it, the probe will
be done. So in this case the order of the driver registration will
dictate the order of probes.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 11/25] OMAPDSS: create custom pdevs for DSS omap_devices
From: Archit Taneja @ 2012-05-04 8:29 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336053481-25433-12-git-send-email-tomi.valkeinen@ti.com>
On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
> Instead of using omap_device_build() to create the omap_devices for DSS
> hwmods, create them with a custom function. This will allow us to create
> a parent-child hierarchy for the devices so that the omapdss_core device
> is parent for the rest of the dss hwmod devices.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> arch/arm/mach-omap2/display.c | 88 ++++++++++++++++++++++++++++++++++-------
> 1 file changed, 74 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 07232fd..46d2a98 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -185,13 +185,71 @@ static int omap_dss_set_min_bus_tput(struct device *dev, unsigned long tput)
> return omap_pm_set_min_bus_tput(dev, OCP_INITIATOR_AGENT, tput);
> }
>
> +static struct platform_device *create_dss_pdev(const char *pdev_name,
> + int pdev_id, const char *oh_name, void *pdata, int pdata_len,
> + struct platform_device *parent)
> +{
> + struct platform_device *pdev;
> + struct omap_device *od;
> + struct omap_hwmod *ohs[1];
> + struct omap_hwmod *oh;
> + int r;
> +
> + oh = omap_hwmod_lookup(oh_name);
> + if (!oh) {
> + pr_err("Could not look up %s\n", oh_name);
> + r = -ENODEV;
> + goto err;
> + }
> +
> + pdev = platform_device_alloc(pdev_name, pdev_id);
> + if (!pdev) {
> + pr_err("Could not create pdev for %s\n", pdev_name);
> + r = -ENOMEM;
> + goto err;
> + }
> +
> + if (parent != NULL)
> + pdev->dev.parent =&parent->dev;
> +
> + if (pdev->id != -1)
> + dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id);
> + else
> + dev_set_name(&pdev->dev, "%s", pdev->name);
> +
> + ohs[0] = oh;
> + od = omap_device_alloc(pdev, ohs, 1, NULL, 0);
> + if (!od) {
> + pr_err("Could not alloc omap_device for %s\n", pdev_name);
> + r = -ENOMEM;
> + goto err;
> + }
> +
> + r = platform_device_add_data(pdev, pdata, pdata_len);
> + if (r) {
> + pr_err("Could not set pdata for %s\n", pdev_name);
> + goto err;
> + }
> +
> + r = omap_device_register(pdev);
> + if (r) {
> + pr_err("Could not register omap_device for %s\n", pdev_name);
> + goto err;
> + }
> +
> + return pdev;
> +
> +err:
> + return ERR_PTR(r);
> +}
> +
> int __init omap_display_init(struct omap_dss_board_info *board_data)
> {
> int r = 0;
> - struct omap_hwmod *oh;
> struct platform_device *pdev;
> int i, oh_count;
> const struct omap_dss_hwmod_data *curr_dss_hwmod;
> + struct platform_device *dss_pdev;
>
> /* create omapdss device */
>
> @@ -221,22 +279,24 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> oh_count = ARRAY_SIZE(omap4_dss_hwmod_data);
> }
>
> - for (i = 0; i< oh_count; i++) {
> - oh = omap_hwmod_lookup(curr_dss_hwmod[i].oh_name);
> - if (!oh) {
> - pr_err("Could not look up %s\n",
> - curr_dss_hwmod[i].oh_name);
> - return -ENODEV;
> - }
> + dss_pdev = NULL;
>
> - pdev = omap_device_build(curr_dss_hwmod[i].dev_name,
> - curr_dss_hwmod[i].id, oh,
> + for (i = 0; i< oh_count; i++) {
> + pdev = create_dss_pdev(curr_dss_hwmod[i].dev_name,
> + curr_dss_hwmod[i].id,
> + curr_dss_hwmod[i].oh_name,
> NULL, 0,
> - NULL, 0, 0);
> + dss_pdev);
> +
> + if (IS_ERR(pdev)) {
> + pr_err("Could not build omap_device for %s\n",
> + curr_dss_hwmod[i].oh_name);
> +
> + return PTR_ERR(pdev);
> + }
>
> - if (WARN((IS_ERR(pdev)), "Could not build omap_device for %s\n",
> - curr_dss_hwmod[i].oh_name))
> - return -ENODEV;
> + if (i = 0)
> + dss_pdev = pdev;
The above line is a bit tricky to understand, maybe something like this
may explain the parent-child setting better:
if (!strcmp(curr_dss_hwmod[i].oh_name, "dss_core"))
dss_pdev = pdev;
I had another general question about the parent-child series. What is
the use of the platform device omap_display_device (with the name
"omapdss"). Is it just a way to get the board data?
Archit
> }
>
> return 0;
^ permalink raw reply
* [PATCH v2] video: exynos_dp: remove unnecessary header includes
From: Jingoo Han @ 2012-05-04 6:25 UTC (permalink / raw)
To: linux-fbdev
Remove unnecessary headers from the file.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 2 --
drivers/video/exynos/exynos_dp_reg.c | 2 --
2 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 6e9f3ce..ef4d1ab 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -21,8 +21,6 @@
#include <video/exynos_dp.h>
-#include <plat/cpu.h>
-
#include "exynos_dp_core.h"
static int exynos_dp_init_dp(struct exynos_dp_device *dp)
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3863726..99bafb8 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -16,8 +16,6 @@
#include <video/exynos_dp.h>
-#include <plat/cpu.h>
-
#include "exynos_dp_core.h"
#include "exynos_dp_reg.h"
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 11/25] OMAPDSS: create custom pdevs for DSS omap_devices
From: Archit Taneja @ 2012-05-04 6:15 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336053481-25433-12-git-send-email-tomi.valkeinen@ti.com>
Hi,
On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
> Instead of using omap_device_build() to create the omap_devices for DSS
> hwmods, create them with a custom function. This will allow us to create
> a parent-child hierarchy for the devices so that the omapdss_core device
> is parent for the rest of the dss hwmod devices.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> arch/arm/mach-omap2/display.c | 88 ++++++++++++++++++++++++++++++++++-------
> 1 file changed, 74 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 07232fd..46d2a98 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -185,13 +185,71 @@ static int omap_dss_set_min_bus_tput(struct device *dev, unsigned long tput)
> return omap_pm_set_min_bus_tput(dev, OCP_INITIATOR_AGENT, tput);
> }
>
> +static struct platform_device *create_dss_pdev(const char *pdev_name,
> + int pdev_id, const char *oh_name, void *pdata, int pdata_len,
> + struct platform_device *parent)
This function looks quite generic, it's like omap_device_build() but
with a parent associated. omap_device_build() seems to be a special case
of this function with parent passed as null. Won't this
function be needed by other devices too?
Maybe we could modify omap_device_build_ss() to take a parent argument,
and make a function called omap_device_build_parent(), where both
omap_device_build() and omap_device_build_parent() call
omap_device_build_ss()?
Archit
> +{
> + struct platform_device *pdev;
> + struct omap_device *od;
> + struct omap_hwmod *ohs[1];
> + struct omap_hwmod *oh;
> + int r;
> +
> + oh = omap_hwmod_lookup(oh_name);
> + if (!oh) {
> + pr_err("Could not look up %s\n", oh_name);
> + r = -ENODEV;
> + goto err;
> + }
> +
> + pdev = platform_device_alloc(pdev_name, pdev_id);
> + if (!pdev) {
> + pr_err("Could not create pdev for %s\n", pdev_name);
> + r = -ENOMEM;
> + goto err;
> + }
> +
> + if (parent != NULL)
> + pdev->dev.parent =&parent->dev;
> +
> + if (pdev->id != -1)
> + dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id);
> + else
> + dev_set_name(&pdev->dev, "%s", pdev->name);
> +
> + ohs[0] = oh;
> + od = omap_device_alloc(pdev, ohs, 1, NULL, 0);
> + if (!od) {
> + pr_err("Could not alloc omap_device for %s\n", pdev_name);
> + r = -ENOMEM;
> + goto err;
> + }
> +
> + r = platform_device_add_data(pdev, pdata, pdata_len);
> + if (r) {
> + pr_err("Could not set pdata for %s\n", pdev_name);
> + goto err;
> + }
> +
> + r = omap_device_register(pdev);
> + if (r) {
> + pr_err("Could not register omap_device for %s\n", pdev_name);
> + goto err;
> + }
> +
> + return pdev;
> +
> +err:
> + return ERR_PTR(r);
> +}
> +
> int __init omap_display_init(struct omap_dss_board_info *board_data)
> {
> int r = 0;
> - struct omap_hwmod *oh;
> struct platform_device *pdev;
> int i, oh_count;
> const struct omap_dss_hwmod_data *curr_dss_hwmod;
> + struct platform_device *dss_pdev;
>
> /* create omapdss device */
>
> @@ -221,22 +279,24 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> oh_count = ARRAY_SIZE(omap4_dss_hwmod_data);
> }
>
> - for (i = 0; i< oh_count; i++) {
> - oh = omap_hwmod_lookup(curr_dss_hwmod[i].oh_name);
> - if (!oh) {
> - pr_err("Could not look up %s\n",
> - curr_dss_hwmod[i].oh_name);
> - return -ENODEV;
> - }
> + dss_pdev = NULL;
>
> - pdev = omap_device_build(curr_dss_hwmod[i].dev_name,
> - curr_dss_hwmod[i].id, oh,
> + for (i = 0; i< oh_count; i++) {
> + pdev = create_dss_pdev(curr_dss_hwmod[i].dev_name,
> + curr_dss_hwmod[i].id,
> + curr_dss_hwmod[i].oh_name,
> NULL, 0,
> - NULL, 0, 0);
> + dss_pdev);
> +
> + if (IS_ERR(pdev)) {
> + pr_err("Could not build omap_device for %s\n",
> + curr_dss_hwmod[i].oh_name);
> +
> + return PTR_ERR(pdev);
> + }
>
> - if (WARN((IS_ERR(pdev)), "Could not build omap_device for %s\n",
> - curr_dss_hwmod[i].oh_name))
> - return -ENODEV;
> + if (i = 0)
> + dss_pdev = pdev;
> }
>
> return 0;
^ permalink raw reply
* Re: [PATCH 08/25] OMAPDSS: clean up the omapdss platform data mess
From: Archit Taneja @ 2012-05-04 5:44 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1336053481-25433-9-git-send-email-tomi.valkeinen@ti.com>
Hi,
On Thursday 03 May 2012 07:27 PM, Tomi Valkeinen wrote:
> The omapdss pdata handling is a mess. This is more evident when trying
> to use device tree for DSS, as we don't have platform data anymore in
> that case. This patch cleans the pdata handling by:
>
> - Remove struct omap_display_platform_data. It was used just as a
> wrapper for struct omap_dss_board_info.
> - Pass the platform data only to omapdss device. The drivers for omap
> dss hwmods do not need the platform data. This should also work better
> for DT, as we can create omapdss device programmatically in generic omap
> boot code, and thus we can pass the pdata to it.
> - Create dss functions for get_ctx_loss_count and dsi_enable/disable_pads
> that the dss hwmod drivers can call.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> arch/arm/mach-omap2/display.c | 39 +++++++++++++++++++--------------------
> drivers/video/omap2/dss/core.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/video/omap2/dss/dispc.c | 21 ++-------------------
> drivers/video/omap2/dss/dsi.c | 17 +++--------------
> drivers/video/omap2/dss/dss.h | 3 +++
> drivers/video/omap2/dss/hdmi.c | 2 --
> include/video/omapdss.h | 5 -----
> 7 files changed, 62 insertions(+), 60 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 60cded4..07232fd 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -191,10 +191,24 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> struct omap_hwmod *oh;
> struct platform_device *pdev;
> int i, oh_count;
> - struct omap_display_platform_data pdata;
> const struct omap_dss_hwmod_data *curr_dss_hwmod;
>
> - memset(&pdata, 0, sizeof(pdata));
> + /* create omapdss device */
> +
> + board_data->dsi_enable_pads = omap_dsi_enable_pads;
> + board_data->dsi_disable_pads = omap_dsi_disable_pads;
> + board_data->get_context_loss_count = omap_pm_get_dev_context_loss_count;
> + board_data->set_min_bus_tput = omap_dss_set_min_bus_tput;
> +
> + omap_display_device.dev.platform_data = board_data;
> +
> + r = platform_device_register(&omap_display_device);
> + if (r< 0) {
> + pr_err("Unable to register omapdss device\n");
> + return r;
> + }
After this patch, the "omapdss" platform device is registered before the
other dss platform devices. This would change the sequence of probes of
these devices. Was this intentional?
Archit
> +
> + /* create devices for dss hwmods */
>
> if (cpu_is_omap24xx()) {
> curr_dss_hwmod = omap2_dss_hwmod_data;
> @@ -207,16 +221,6 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> oh_count = ARRAY_SIZE(omap4_dss_hwmod_data);
> }
>
> - if (board_data->dsi_enable_pads = NULL)
> - board_data->dsi_enable_pads = omap_dsi_enable_pads;
> - if (board_data->dsi_disable_pads = NULL)
> - board_data->dsi_disable_pads = omap_dsi_disable_pads;
> -
> - pdata.board_data = board_data;
> - pdata.board_data->get_context_loss_count > - omap_pm_get_dev_context_loss_count;
> - pdata.board_data->set_min_bus_tput = omap_dss_set_min_bus_tput;
> -
> for (i = 0; i< oh_count; i++) {
> oh = omap_hwmod_lookup(curr_dss_hwmod[i].oh_name);
> if (!oh) {
> @@ -226,21 +230,16 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
> }
>
> pdev = omap_device_build(curr_dss_hwmod[i].dev_name,
> - curr_dss_hwmod[i].id, oh,&pdata,
> - sizeof(struct omap_display_platform_data),
> + curr_dss_hwmod[i].id, oh,
> + NULL, 0,
> NULL, 0, 0);
>
> if (WARN((IS_ERR(pdev)), "Could not build omap_device for %s\n",
> curr_dss_hwmod[i].oh_name))
> return -ENODEV;
> }
> - omap_display_device.dev.platform_data = board_data;
>
> - r = platform_device_register(&omap_display_device);
> - if (r< 0)
> - printk(KERN_ERR "Unable to register OMAP-Display device\n");
> -
> - return r;
> + return 0;
> }
>
> static void dispc_disable_outputs(void)
> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> index 64cb8aa..b37b6f4 100644
> --- a/drivers/video/omap2/dss/core.c
> +++ b/drivers/video/omap2/dss/core.c
> @@ -87,6 +87,41 @@ struct regulator *dss_get_vdds_sdi(void)
> return reg;
> }
>
> +int dss_get_ctx_loss_count(struct device *dev)
> +{
> + struct omap_dss_board_info *board_data = core.pdev->dev.platform_data;
> + int cnt;
> +
> + if (!board_data->get_context_loss_count)
> + return -ENOENT;
> +
> + cnt = board_data->get_context_loss_count(dev);
> +
> + WARN_ONCE(cnt< 0, "get_context_loss_count failed: %d\n", cnt);
> +
> + return cnt;
> +}
> +
> +int dss_dsi_enable_pads(int dsi_id, unsigned lane_mask)
> +{
> + struct omap_dss_board_info *board_data = core.pdev->dev.platform_data;
> +
> + if (!board_data->dsi_enable_pads)
> + return -ENOENT;
> +
> + return board_data->dsi_enable_pads(dsi_id, lane_mask);
> +}
> +
> +void dss_dsi_disable_pads(int dsi_id, unsigned lane_mask)
> +{
> + struct omap_dss_board_info *board_data = core.pdev->dev.platform_data;
> +
> + if (!board_data->dsi_enable_pads)
> + return;
> +
> + return board_data->dsi_disable_pads(dsi_id, lane_mask);
> +}
> +
> int dss_set_min_bus_tput(struct device *dev, unsigned long tput)
> {
> struct omap_dss_board_info *pdata = core.pdev->dev.platform_data;
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 49015b8..262ed29 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -131,23 +131,6 @@ static inline u32 dispc_read_reg(const u16 idx)
> return __raw_readl(dispc.base + idx);
> }
>
> -static int dispc_get_ctx_loss_count(void)
> -{
> - struct device *dev =&dispc.pdev->dev;
> - struct omap_display_platform_data *pdata = dev->platform_data;
> - struct omap_dss_board_info *board_data = pdata->board_data;
> - int cnt;
> -
> - if (!board_data->get_context_loss_count)
> - return -ENOENT;
> -
> - cnt = board_data->get_context_loss_count(dev);
> -
> - WARN_ONCE(cnt< 0, "get_context_loss_count failed: %d\n", cnt);
> -
> - return cnt;
> -}
> -
> #define SR(reg) \
> dispc.ctx[DISPC_##reg / sizeof(u32)] = dispc_read_reg(DISPC_##reg)
> #define RR(reg) \
> @@ -251,7 +234,7 @@ static void dispc_save_context(void)
> if (dss_has_feature(FEAT_CORE_CLK_DIV))
> SR(DIVISOR);
>
> - dispc.ctx_loss_cnt = dispc_get_ctx_loss_count();
> + dispc.ctx_loss_cnt = dss_get_ctx_loss_count(&dispc.pdev->dev);
> dispc.ctx_valid = true;
>
> DSSDBG("context saved, ctx_loss_count %d\n", dispc.ctx_loss_cnt);
> @@ -266,7 +249,7 @@ static void dispc_restore_context(void)
> if (!dispc.ctx_valid)
> return;
>
> - ctx = dispc_get_ctx_loss_count();
> + ctx = dss_get_ctx_loss_count(&dispc.pdev->dev);
>
> if (ctx>= 0&& ctx = dispc.ctx_loss_cnt)
> return;
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index f6ecc3a..a5a8316 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -261,9 +261,6 @@ struct dsi_data {
> struct clk *dss_clk;
> struct clk *sys_clk;
>
> - int (*enable_pads)(int dsi_id, unsigned lane_mask);
> - void (*disable_pads)(int dsi_id, unsigned lane_mask);
> -
> struct dsi_clock_info current_cinfo;
>
> bool vdds_dsi_enabled;
> @@ -2365,7 +2362,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
>
> DSSDBGF();
>
> - r = dsi->enable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> + r = dss_dsi_enable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> if (r)
> return r;
>
> @@ -2475,21 +2472,20 @@ err_cio_pwr:
> dsi_cio_disable_lane_override(dsidev);
> err_scp_clk_dom:
> dsi_disable_scp_clk(dsidev);
> - dsi->disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> + dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> return r;
> }
>
> static void dsi_cio_uninit(struct omap_dss_device *dssdev)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> - struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
>
> /* DDR_CLK_ALWAYS_ON */
> REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 0, 13, 13);
>
> dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_OFF);
> dsi_disable_scp_clk(dsidev);
> - dsi->disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> + dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev));
> }
>
> static void dsi_config_tx_fifo(struct platform_device *dsidev,
> @@ -4647,8 +4643,6 @@ static void dsi_put_clocks(struct platform_device *dsidev)
> /* DSI1 HW IP initialisation */
> static int omap_dsihw_probe(struct platform_device *dsidev)
> {
> - struct omap_display_platform_data *dss_plat_data;
> - struct omap_dss_board_info *board_info;
> u32 rev;
> int r, i, dsi_module = dsi_get_dsidev_id(dsidev);
> struct resource *dsi_mem;
> @@ -4662,11 +4656,6 @@ static int omap_dsihw_probe(struct platform_device *dsidev)
> dsi_pdev_map[dsi_module] = dsidev;
> dev_set_drvdata(&dsidev->dev, dsi);
>
> - dss_plat_data = dsidev->dev.platform_data;
> - board_info = dss_plat_data->board_data;
> - dsi->enable_pads = board_info->dsi_enable_pads;
> - dsi->disable_pads = board_info->dsi_disable_pads;
> -
> spin_lock_init(&dsi->irq_lock);
> spin_lock_init(&dsi->errors_lock);
> dsi->errors = 0;
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 5ca67f1..bb3079d 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -159,6 +159,9 @@ struct platform_device;
> struct bus_type *dss_get_bus(void);
> struct regulator *dss_get_vdds_dsi(void);
> struct regulator *dss_get_vdds_sdi(void);
> +int dss_get_ctx_loss_count(struct device *dev);
> +int dss_dsi_enable_pads(int dsi_id, unsigned lane_mask);
> +void dss_dsi_disable_pads(int dsi_id, unsigned lane_mask);
> int dss_set_min_bus_tput(struct device *dev, unsigned long tput);
>
> /* apply */
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 56f6e9c..2321b75 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -63,7 +63,6 @@
>
> static struct {
> struct mutex lock;
> - struct omap_display_platform_data *pdata;
> struct platform_device *pdev;
> struct hdmi_ip_data ip_data;
>
> @@ -795,7 +794,6 @@ static int omapdss_hdmihw_probe(struct platform_device *pdev)
> struct resource *hdmi_mem;
> int r;
>
> - hdmi.pdata = pdev->dev.platform_data;
> hdmi.pdev = pdev;
>
> mutex_init(&hdmi.lock);
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 5f36ddd..49e7073 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -317,11 +317,6 @@ extern int omap_display_init(struct omap_dss_board_info *board_data);
> /* HDMI mux init*/
> extern int omap_hdmi_init(enum omap_hdmi_flags flags);
>
> -struct omap_display_platform_data {
> - struct omap_dss_board_info *board_data;
> - /* TODO: Additional members to be added when PM is considered */
> -};
> -
> struct omap_video_timings {
> /* Unit: pixels */
> u16 x_res;
^ permalink raw reply
* [PATCH] video: EXYNOS: enable interrupt again after sw reset
From: Donghwa Lee @ 2012-05-04 4:59 UTC (permalink / raw)
To: linux-arm-kernel
When exynos_mipi_update_cfg() is called, mipi dsi registers were become
sw reset. So, It needs to enable interrupt again.
Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/video/exynos/exynos_mipi_dsi.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index 557091d..49c7351 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -102,6 +102,8 @@ static void exynos_mipi_update_cfg(struct mipi_dsim_device *dsim)
/* set display timing. */
exynos_mipi_dsi_set_display_mode(dsim, dsim->dsim_config);
+ exynos_mipi_dsi_init_interrupt(dsim);
+
/*
* data from Display controller(FIMD) is transferred in video mode
* but in case of command mode, all settigs is updated to registers.
--
1.7.4.1
^ permalink raw reply related
* [PATCH] video: exynos_dp: fix max loop count in EQ sequence of link training
From: Jingoo Han @ 2012-05-04 1:49 UTC (permalink / raw)
To: linux-fbdev
This patch fixes max loop count in EQ(Channel Equalization) sequence
of link training. According to DP(displayport) specification,
the max loop count in this sequence should be 5.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
include/video/exynos_dp.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/video/exynos_dp.h b/include/video/exynos_dp.h
index 8847a9d..bd8cabd 100644
--- a/include/video/exynos_dp.h
+++ b/include/video/exynos_dp.h
@@ -14,7 +14,7 @@
#define DP_TIMEOUT_LOOP_COUNT 100
#define MAX_CR_LOOP 5
-#define MAX_EQ_LOOP 4
+#define MAX_EQ_LOOP 5
enum link_rate_type {
LINK_RATE_1_62GBPS = 0x06,
--
1.7.1
^ permalink raw reply related
* [PATCH] video: exynos_dp: remove unnecessary header includes
From: Jingoo Han @ 2012-05-04 1:48 UTC (permalink / raw)
To: linux-fbdev
Remove unnecessary headers from the file.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_reg.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
index 3863726..99bafb8 100644
--- a/drivers/video/exynos/exynos_dp_reg.c
+++ b/drivers/video/exynos/exynos_dp_reg.c
@@ -16,8 +16,6 @@
#include <video/exynos_dp.h>
-#include <plat/cpu.h>
-
#include "exynos_dp_core.h"
#include "exynos_dp_reg.h"
--
1.7.1
^ permalink raw reply related
* [PATCH v2] video/via: Convert to kstrtou8_from_user
From: Peter Huewe @ 2012-05-04 0:23 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Alexey Dobriyan, linux-fbdev, linux-kernel, kernel-janitors,
Peter Huewe
In-Reply-To: <CACVxJT96rXRqMoeabdDyNEjXCubuAnQ_fonisbopQvYL=Qhp2w@mail.gmail.com>
This patch replaces the code for getting an number from a
userspace buffer by a simple call to kstrou8_from_user.
This makes it easier to read and less error prone.
v2:
removed initialization of reg_val and dropped check if count < 1
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
@Alexey: Thanks for the review/feedback!
drivers/video/via/viafbdev.c | 34 ++++++++++++----------------------
1 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index a13c258..88bf5ba 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1279,17 +1279,12 @@ static int viafb_dfph_proc_open(struct inode *inode, struct file *file)
static ssize_t viafb_dfph_proc_write(struct file *file,
const char __user *buffer, size_t count, loff_t *pos)
{
- char buf[20];
- u8 reg_val = 0;
- unsigned long length;
- if (count < 1)
- return -EINVAL;
- length = count > 20 ? 20 : count;
- if (copy_from_user(&buf[0], buffer, length))
- return -EFAULT;
- buf[length - 1] = '\0'; /*Ensure end string */
- if (kstrtou8(buf, 0, ®_val) < 0)
- return -EINVAL;
+ int err;
+ u8 reg_val;
+ err = kstrtou8_from_user(buffer, count, 0, ®_val);
+ if (err)
+ return err;
+
viafb_write_reg_mask(CR97, VIACR, reg_val, 0x0f);
return count;
}
@@ -1319,17 +1314,12 @@ static int viafb_dfpl_proc_open(struct inode *inode, struct file *file)
static ssize_t viafb_dfpl_proc_write(struct file *file,
const char __user *buffer, size_t count, loff_t *pos)
{
- char buf[20];
- u8 reg_val = 0;
- unsigned long length;
- if (count < 1)
- return -EINVAL;
- length = count > 20 ? 20 : count;
- if (copy_from_user(&buf[0], buffer, length))
- return -EFAULT;
- buf[length - 1] = '\0'; /*Ensure end string */
- if (kstrtou8(buf, 0, ®_val) < 0)
- return -EINVAL;
+ int err;
+ u8 reg_val;
+ err = kstrtou8_from_user(buffer, count, 0, ®_val);
+ if (err)
+ return err;
+
viafb_write_reg_mask(CR99, VIACR, reg_val, 0x0f);
return count;
}
--
1.7.3.4
^ permalink raw reply related
* [PATCH] video/sis: Annotate SiS_DRAMType as __devinitconst
From: Peter Huewe @ 2012-05-03 22:14 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Thomas Winischhofer, linux-fbdev, linux-kernel, Peter Huewe
In-Reply-To: <4FA2FD10.6090306@gmx.de>
SiS_DRAMType is const and only used by sisfb_post_300_rwtest which is
marked __devinit we can annotate SiS_DRAMType with __devinitconst and
move it into the file scope in order to not have it created on the
stack.
This patch decreases the compiled module size by about 100bytes.
And since hardcoded values are bad we use ARRAY_SIZE for determining
the size of SiS_DRAMType ;)
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/video/sis/sis_main.c | 41 +++++++++++++++++++++--------------------
1 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
index 078ca21..a7a48db 100644
--- a/drivers/video/sis/sis_main.c
+++ b/drivers/video/sis/sis_main.c
@@ -4222,6 +4222,26 @@ sisfb_post_300_buswidth(struct sis_video_info *ivideo)
return 1; /* 32bit */
}
+static const unsigned short __devinitconst SiS_DRAMType[17][5] = {
+ {0x0C,0x0A,0x02,0x40,0x39},
+ {0x0D,0x0A,0x01,0x40,0x48},
+ {0x0C,0x09,0x02,0x20,0x35},
+ {0x0D,0x09,0x01,0x20,0x44},
+ {0x0C,0x08,0x02,0x10,0x31},
+ {0x0D,0x08,0x01,0x10,0x40},
+ {0x0C,0x0A,0x01,0x20,0x34},
+ {0x0C,0x09,0x01,0x08,0x32},
+ {0x0B,0x08,0x02,0x08,0x21},
+ {0x0C,0x08,0x01,0x08,0x30},
+ {0x0A,0x08,0x02,0x04,0x11},
+ {0x0B,0x0A,0x01,0x10,0x28},
+ {0x09,0x08,0x02,0x02,0x01},
+ {0x0B,0x09,0x01,0x08,0x24},
+ {0x0B,0x08,0x01,0x04,0x20},
+ {0x0A,0x08,0x01,0x02,0x10},
+ {0x09,0x08,0x01,0x01,0x00}
+};
+
static int __devinit
sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration, int buswidth,
int PseudoRankCapacity, int PseudoAdrPinCount,
@@ -4231,27 +4251,8 @@ sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration, int buswidth
unsigned short sr14;
unsigned int k, RankCapacity, PageCapacity, BankNumHigh, BankNumMid;
unsigned int PhysicalAdrOtherPage, PhysicalAdrHigh, PhysicalAdrHalfPage;
- static const unsigned short SiS_DRAMType[17][5] = {
- {0x0C,0x0A,0x02,0x40,0x39},
- {0x0D,0x0A,0x01,0x40,0x48},
- {0x0C,0x09,0x02,0x20,0x35},
- {0x0D,0x09,0x01,0x20,0x44},
- {0x0C,0x08,0x02,0x10,0x31},
- {0x0D,0x08,0x01,0x10,0x40},
- {0x0C,0x0A,0x01,0x20,0x34},
- {0x0C,0x09,0x01,0x08,0x32},
- {0x0B,0x08,0x02,0x08,0x21},
- {0x0C,0x08,0x01,0x08,0x30},
- {0x0A,0x08,0x02,0x04,0x11},
- {0x0B,0x0A,0x01,0x10,0x28},
- {0x09,0x08,0x02,0x02,0x01},
- {0x0B,0x09,0x01,0x08,0x24},
- {0x0B,0x08,0x01,0x04,0x20},
- {0x0A,0x08,0x01,0x02,0x10},
- {0x09,0x08,0x01,0x01,0x00}
- };
- for(k = 0; k <= 16; k++) {
+ for(k = 0; k < ARRAY_SIZE(SiS_DRAMType); k++) {
RankCapacity = buswidth * SiS_DRAMType[k][3];
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH v2] video/sis: Use SiS_DRAMType from init.h and annotate it __devinitconst
From: Peter Hüwe @ 2012-05-03 21:58 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: Thomas Winischhofer, linux-fbdev, linux-kernel
In-Reply-To: <4FA2FD10.6090306@gmx.de>
Am Donnerstag 03 Mai 2012, 23:48:00 schrieb Florian Tobias Schandinat:
> You can still mark it as __devinitconst, I just wanted to have the array
> inside a C file, not a header. If you agree with this, I'll change your
> new patch that way.
I'm totally fine with this - Thanks,
Peter
^ permalink raw reply
* Re: [PATCH v2] video/sis: Use SiS_DRAMType from init.h and annotate it __devinitconst
From: Florian Tobias Schandinat @ 2012-05-03 21:48 UTC (permalink / raw)
To: Peter Hüwe; +Cc: Thomas Winischhofer, linux-fbdev, linux-kernel
In-Reply-To: <201205032340.34483.PeterHuewe@gmx.de>
Hi Peter,
On 05/03/2012 09:40 PM, Peter Hüwe wrote:
> Hi Florian,
>
>> As far as I can see it should be possible to keep the array in
>> sis_main.c and just delete it in the header files, shouldn't it? I'd
>> prefer to do it this way.
>
> Yes it is absolutely possible to leave it in sis_main.c and remove it from the
> header files. I already thought about this when creating the patch, but
> decided against it, as the 17*5 = 85 bytes are allocated on the stack, while
> they nicely can be put in the .devinit.rodata section ;)
You can still mark it as __devinitconst, I just wanted to have the array
inside a C file, not a header. If you agree with this, I'll change your
new patch that way.
>
> With the patch:
> 344072 sis_main.o
> 1200950 sisfb.o
> 1217491 sisfb.ko
>
> vs without the patch and removing it only from the header:
> 344176 sis_main.o
> 1201056 sisfb.o
> 1217597 sisfb.ko -> ~100bytes more in the final module.
>
> However I'm fine with this and will remove it from the header and squash this
> together with the
> "video/sis: Remove unused structs SiS_SDRDRAM_TYPE/SiS_DDRDRAM_TYPE"
> I also sent to you.
Thanks,
Florian Tobias Schandinat
^ permalink raw reply
* [PATCH] video/sis: Remove unused structs SiS_SDRDRAM_TYPE/SiS_DDRDRAM_TYPE/SiS_DRAMType
From: Peter Huewe @ 2012-05-03 21:42 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Thomas Winischhofer, linux-fbdev, linux-kernel, Peter Huewe
This patch removes the unused structs SiS_SDRDRAM_TYPE and SiS_DDRDRAM_TYPE
from init.h
These are not used anywhere so we can delete them.
The SiS_DRAMType is identically defined in sis_main.c and only used
there so we can remove it here.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/video/sis/init.h | 45 ---------------------------------------------
1 files changed, 0 insertions(+), 45 deletions(-)
diff --git a/drivers/video/sis/init.h b/drivers/video/sis/init.h
index aff7384..85d6738 100644
--- a/drivers/video/sis/init.h
+++ b/drivers/video/sis/init.h
@@ -105,51 +105,6 @@ static const unsigned short ModeIndex_1920x1440[] = {0x68, 0x69, 0x00, 0x6b};
static const unsigned short ModeIndex_300_2048x1536[]= {0x6c, 0x6d, 0x00, 0x00};
static const unsigned short ModeIndex_310_2048x1536[]= {0x6c, 0x6d, 0x00, 0x6e};
-static const unsigned short SiS_DRAMType[17][5]={
- {0x0C,0x0A,0x02,0x40,0x39},
- {0x0D,0x0A,0x01,0x40,0x48},
- {0x0C,0x09,0x02,0x20,0x35},
- {0x0D,0x09,0x01,0x20,0x44},
- {0x0C,0x08,0x02,0x10,0x31},
- {0x0D,0x08,0x01,0x10,0x40},
- {0x0C,0x0A,0x01,0x20,0x34},
- {0x0C,0x09,0x01,0x08,0x32},
- {0x0B,0x08,0x02,0x08,0x21},
- {0x0C,0x08,0x01,0x08,0x30},
- {0x0A,0x08,0x02,0x04,0x11},
- {0x0B,0x0A,0x01,0x10,0x28},
- {0x09,0x08,0x02,0x02,0x01},
- {0x0B,0x09,0x01,0x08,0x24},
- {0x0B,0x08,0x01,0x04,0x20},
- {0x0A,0x08,0x01,0x02,0x10},
- {0x09,0x08,0x01,0x01,0x00}
-};
-
-static const unsigned short SiS_SDRDRAM_TYPE[13][5] -{
- { 2,12, 9,64,0x35},
- { 1,13, 9,64,0x44},
- { 2,12, 8,32,0x31},
- { 2,11, 9,32,0x25},
- { 1,12, 9,32,0x34},
- { 1,13, 8,32,0x40},
- { 2,11, 8,16,0x21},
- { 1,12, 8,16,0x30},
- { 1,11, 9,16,0x24},
- { 1,11, 8, 8,0x20},
- { 2, 9, 8, 4,0x01},
- { 1,10, 8, 4,0x10},
- { 1, 9, 8, 2,0x00}
-};
-
-static const unsigned short SiS_DDRDRAM_TYPE[4][5] -{
- { 2,12, 9,64,0x35},
- { 2,12, 8,32,0x31},
- { 2,11, 8,16,0x21},
- { 2, 9, 8, 4,0x01}
-};
-
static const unsigned char SiS_MDA_DAC[] {
0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH v2] video/sis: Use SiS_DRAMType from init.h and annotate it __devinitconst
From: Peter Hüwe @ 2012-05-03 21:40 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: Thomas Winischhofer, linux-fbdev, linux-kernel
In-Reply-To: <4FA1BF76.8080109@gmx.de>
Hi Florian,
> As far as I can see it should be possible to keep the array in
> sis_main.c and just delete it in the header files, shouldn't it? I'd
> prefer to do it this way.
Yes it is absolutely possible to leave it in sis_main.c and remove it from the
header files. I already thought about this when creating the patch, but
decided against it, as the 17*5 = 85 bytes are allocated on the stack, while
they nicely can be put in the .devinit.rodata section ;)
With the patch:
344072 sis_main.o
1200950 sisfb.o
1217491 sisfb.ko
vs without the patch and removing it only from the header:
344176 sis_main.o
1201056 sisfb.o
1217597 sisfb.ko -> ~100bytes more in the final module.
However I'm fine with this and will remove it from the header and squash this
together with the
"video/sis: Remove unused structs SiS_SDRDRAM_TYPE/SiS_DDRDRAM_TYPE"
I also sent to you.
Thanks,
Peterq
^ permalink raw reply
* Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING
From: Jean Delvare @ 2012-05-03 18:36 UTC (permalink / raw)
To: Mark Brown
Cc: Florian Tobias Schandinat, Dmitry Torokhov, Wolfram Sang,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1336042416-28330-1-git-send-email-broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Hi Mark,
On Thu, 3 May 2012 11:53:36 +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
This is all correct, but it should be documented in
Documentation/i2c/i2c-protocol. At the moment documentation still says
that I2C_M_NOSTART is a weird protocol quirk nobody should be using.
When you update the documentation, I think it is important to stress
that there are now two use cases of I2C_M_NOSTART. If direction
changes, it is a rarely needed protocol quirk. If direction doesn't
change, it is used for buffer gathering.
>
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> Documentation/i2c/functionality | 1 +
> drivers/base/regmap/regmap-i2c.c | 2 +-
> drivers/i2c/algos/i2c-algo-bit.c | 2 +-
> drivers/i2c/algos/i2c-algo-pcf.c | 2 +-
> drivers/i2c/busses/i2c-nuc900.c | 3 ++-
> drivers/i2c/busses/i2c-s3c2410.c | 3 ++-
> drivers/input/joystick/as5011.c | 1 +
> drivers/video/matrox/matroxfb_maven.c | 1 +
> include/linux/i2c.h | 5 +++--
> 9 files changed, 13 insertions(+), 7 deletions(-)
Review:
>
> diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
> index 42c17c1..fb2e77e 100644
> --- a/Documentation/i2c/functionality
> +++ b/Documentation/i2c/functionality
> @@ -33,6 +33,7 @@ For the most up-to-date list of functionality constants, please check
You must also update the description of I2C_FUNC_PROTOCOL_MANGLING to
no longer mention I2C_M_NOSTART.
> I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
> I2C_FUNC_SMBUS_READ_I2C_BLOCK Handles the SMBus read_i2c_block_data command
> I2C_FUNC_SMBUS_WRITE_I2C_BLOCK Handles the SMBus write_i2c_block_data command
> + I2C_FUNC_NOSTART Transfers can be sent without a start
Not true. Transfers always begin with a Start, even when I2C_M_NOSTART
is used, because it is never set for the first message.
I2C_FUNC_NOSTART means that messages other than the first may not get
their usual Repeated Start and address prefix. So a better working IMHO
would be:
+ I2C_FUNC_NOSTART Can skip repeated start sequence
>
> A few combinations of the above flags are also defined for your convenience:
>
> diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
> index 5f6b247..fa6bf52 100644
> --- a/drivers/base/regmap/regmap-i2c.c
> +++ b/drivers/base/regmap/regmap-i2c.c
> @@ -42,7 +42,7 @@ static int regmap_i2c_gather_write(void *context,
> /* If the I2C controller can't do a gather tell the core, it
> * will substitute in a linear write for us.
> */
> - if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
> + if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
> return -ENOTSUPP;
>
> xfer[0].addr = i2c->addr;
> diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
> index 7f0b832..fad22b0 100644
> --- a/drivers/i2c/algos/i2c-algo-bit.c
> +++ b/drivers/i2c/algos/i2c-algo-bit.c
> @@ -608,7 +608,7 @@ bailout:
>
> static u32 bit_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> + return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_SMBUS_READ_BLOCK_DATA |
> I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
> I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
> diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
> index 5c23795..8b38986 100644
> --- a/drivers/i2c/algos/i2c-algo-pcf.c
> +++ b/drivers/i2c/algos/i2c-algo-pcf.c
> @@ -401,7 +401,7 @@ out:
>
> static u32 pcf_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> + return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
> I2C_FUNC_PROTOCOL_MANGLING;
> }
>
A quick grep suggests that i2c-algo-pcf doesn't support I2C_M_NOSTART,
only I2C_M_REV_DIR_ADDR. So you don't want to add I2C_FUNC_NOSTART to
the functionality mask.
> diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
> index 03b6157..a26dfb8 100644
> --- a/drivers/i2c/busses/i2c-nuc900.c
> +++ b/drivers/i2c/busses/i2c-nuc900.c
> @@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
> /* declare our i2c functionality */
> static u32 nuc900_i2c_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
> + I2C_FUNC_PROTOCOL_MANGLING;
> }
>
> /* i2c bus registration info */
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index fa0b134..0195915 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -626,7 +626,8 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
> /* declare our i2c functionality */
> static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
> {
> - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
> + I2C_FUNC_PROTOCOL_MANGLING;
> }
>
> /* i2c bus registration info */
> diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
> index 3063464..57d19d4 100644
> --- a/drivers/input/joystick/as5011.c
> +++ b/drivers/input/joystick/as5011.c
> @@ -231,6 +231,7 @@ static int __devinit as5011_probe(struct i2c_client *client,
> }
>
> if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_NOSTART |
> I2C_FUNC_PROTOCOL_MANGLING)) {
> dev_err(&client->dev,
> "need i2c bus that supports protocol mangling\n");
> diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
> index 31b8f67..217678e 100644
> --- a/drivers/video/matrox/matroxfb_maven.c
> +++ b/drivers/video/matrox/matroxfb_maven.c
> @@ -1243,6 +1243,7 @@ static int maven_probe(struct i2c_client *client,
>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
> I2C_FUNC_SMBUS_BYTE_DATA |
> + I2C_FUNC_NOSTART |
> I2C_FUNC_PROTOCOL_MANGLING))
> goto ERROR0;
> if (!(data = kzalloc(sizeof(*data), GFP_KERNEL))) {
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b66cb60..da46925 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -541,7 +541,7 @@ struct i2c_msg {
> __u16 flags;
> #define I2C_M_TEN 0x0010 /* this is a ten bit chip address */
> #define I2C_M_RD 0x0001 /* read data, from slave to master */
> -#define I2C_M_NOSTART 0x4000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NOSTART 0x4000 /* if I2C_FUNC_NOSTART */
> #define I2C_M_REV_DIR_ADDR 0x2000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_IGNORE_NAK 0x1000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_NO_RD_ACK 0x0800 /* if I2C_FUNC_PROTOCOL_MANGLING */
> @@ -554,7 +554,7 @@ struct i2c_msg {
>
> #define I2C_FUNC_I2C 0x00000001
> #define I2C_FUNC_10BIT_ADDR 0x00000002
> -#define I2C_FUNC_PROTOCOL_MANGLING 0x00000004 /* I2C_M_NOSTART etc. */
> +#define I2C_FUNC_PROTOCOL_MANGLING 0x00000004 /* I2C_M_IGNORE_NAK etc. */
> #define I2C_FUNC_SMBUS_PEC 0x00000008
> #define I2C_FUNC_SMBUS_BLOCK_PROC_CALL 0x00008000 /* SMBus 2.0 */
> #define I2C_FUNC_SMBUS_QUICK 0x00010000
> @@ -569,6 +569,7 @@ struct i2c_msg {
> #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x02000000
> #define I2C_FUNC_SMBUS_READ_I2C_BLOCK 0x04000000 /* I2C-like block xfer */
> #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK 0x08000000 /* w/ 1-byte reg. addr. */
> +#define I2C_FUNC_NOSTART 0x10000000 /* I2C_M_NOSTART */
Sorry for nitpicking but wouldn't I2C_FUNC_GATHER be a better name?
NOSTART is an implementation detail now, the high level feature is the
ability to gather multiple messages into one.
Also I think I would prefer if you used 0x00000010 for the new
functionality, so that it isn't too far away from
I2C_FUNC_PROTOCOL_MANGLING.
>
> #define I2C_FUNC_SMBUS_BYTE (I2C_FUNC_SMBUS_READ_BYTE | \
> I2C_FUNC_SMBUS_WRITE_BYTE)
Thanks,
--
Jean Delvare
^ permalink raw reply
* Re: [PATCH v2 1/4] mfd: add LM3533 lighting-power core driver
From: Johan Hovold @ 2012-05-03 17:31 UTC (permalink / raw)
To: Mark Brown
Cc: Rob Landley, Richard Purdie, Samuel Ortiz, Jonathan Cameron,
Greg Kroah-Hartman, Florian Tobias Schandinat, Arnd Bergmann,
Andrew Morton, linux-doc, linux-kernel, linux-iio, devel,
linux-fbdev
In-Reply-To: <20120503172309.GT3955@opensource.wolfsonmicro.com>
On Thu, May 03, 2012 at 06:23:10PM +0100, Mark Brown wrote:
> On Thu, May 03, 2012 at 07:14:02PM +0200, Johan Hovold wrote:
>
> > Assigning a current sink to a control bank corresponds, then, to
> > setting, for example, output_lvled3 to (led) 1.
>
> This seems sensible enough, though it does feel like what we're offering
> up to the LED subsystem is relly the LED control banks rather than the
> LEDs themselves - or some mix of this and other stuff.
Exactly. That's why I've tried to refer to the led devices as "logical
leds" as they may be connected to more than one output (the physical
leds).
Johan
^ permalink raw reply
* Re: [PATCH v2 1/4] mfd: add LM3533 lighting-power core driver
From: Mark Brown @ 2012-05-03 17:23 UTC (permalink / raw)
To: Johan Hovold
Cc: Rob Landley, Richard Purdie, Samuel Ortiz, Jonathan Cameron,
Greg Kroah-Hartman, Florian Tobias Schandinat, Arnd Bergmann,
Andrew Morton, linux-doc, linux-kernel, linux-iio, devel,
linux-fbdev
In-Reply-To: <20120503171402.GG15752@localhost>
[-- Attachment #1: Type: text/plain, Size: 380 bytes --]
On Thu, May 03, 2012 at 07:14:02PM +0200, Johan Hovold wrote:
> Assigning a current sink to a control bank corresponds, then, to
> setting, for example, output_lvled3 to (led) 1.
This seems sensible enough, though it does feel like what we're offering
up to the LED subsystem is relly the LED control banks rather than the
LEDs themselves - or some mix of this and other stuff.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] mfd: add LM3533 lighting-power core driver
From: Johan Hovold @ 2012-05-03 17:14 UTC (permalink / raw)
To: Mark Brown
Cc: Rob Landley, Richard Purdie, Samuel Ortiz, Jonathan Cameron,
Greg Kroah-Hartman, Florian Tobias Schandinat, Arnd Bergmann,
Andrew Morton, linux-doc, linux-kernel, linux-iio, devel,
linux-fbdev
In-Reply-To: <20120503165748.GS3955@opensource.wolfsonmicro.com>
On Thu, May 03, 2012 at 05:57:49PM +0100, Mark Brown wrote:
> On Thu, May 03, 2012 at 06:54:37PM +0200, Johan Hovold wrote:
> > On Thu, May 03, 2012 at 04:24:07PM +0100, Mark Brown wrote:
>
> > > I'm not sure exactly which control that is?
>
> > That would be the output_lvled[n] (n = 1..5) attributes. For example, to
> > have all five low-voltage sinks blink synchronously, you could assign 0
> > to all these five attributes, and set a timer trigger for the led device
> > which has id 0.
>
> Sorry, I meant "what exactly does this do in hardware".
From the datasheet (page 14):
"CONTROL BANK MAPPING
Control of the LM3533's current sinks is not done directly, but
through the programming of Control Banks. The current sinks are
then assigned to the programmed Control Bank. This allows for
a wide variety of current control possibilities where LEDs can
be grouped and controlled via specific Control Banks (see
Figure 3)."
It is the control banks that has a brightness settings or can be
programmed to blink, that is, they correspond to the logical LEDs and
backlights.
Assigning a current sink to a control bank corresponds, then, to
setting, for example, output_lvled3 to (led) 1.
[ Figure 3 on page 16 of the data sheet may be instructive. In the
figure, BANK A and B corresponds to the two backlight devices, and
BANK C through F corresponds to the four led devices. Note that there
are more outputs (current sinks) than control banks. ]
Thanks,
Johan
^ permalink raw reply
* Re: [PATCH v2 1/4] mfd: add LM3533 lighting-power core driver
From: Mark Brown @ 2012-05-03 16:57 UTC (permalink / raw)
To: Johan Hovold
Cc: Rob Landley, Richard Purdie, Samuel Ortiz, Jonathan Cameron,
Greg Kroah-Hartman, Florian Tobias Schandinat, Arnd Bergmann,
Andrew Morton, linux-doc, linux-kernel, linux-iio, devel,
linux-fbdev
In-Reply-To: <20120503165437.GF15752@localhost>
[-- Attachment #1: Type: text/plain, Size: 473 bytes --]
On Thu, May 03, 2012 at 06:54:37PM +0200, Johan Hovold wrote:
> On Thu, May 03, 2012 at 04:24:07PM +0100, Mark Brown wrote:
> > I'm not sure exactly which control that is?
> That would be the output_lvled[n] (n = 1..5) attributes. For example, to
> have all five low-voltage sinks blink synchronously, you could assign 0
> to all these five attributes, and set a timer trigger for the led device
> which has id 0.
Sorry, I meant "what exactly does this do in hardware".
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ 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