* [PATCH v2 1/7] OMAPDSS: HDMI: Rename resource variable at probe.
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 2/7] OMAPDSS: HDMI: Convert to devm_request_and_ioremap Ricardo Neri
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
Minor cleanup to give to the resource variable a more proper name.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/hdmi.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index adcc906..cdb043d 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -998,22 +998,21 @@ static void __exit hdmi_uninit_output(struct platform_device *pdev)
/* HDMI HW IP initialisation */
static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
{
- struct resource *hdmi_mem;
+ struct resource *res;
int r;
hdmi.pdev = pdev;
mutex_init(&hdmi.lock);
- hdmi_mem = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
- if (!hdmi_mem) {
+ res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
+ if (!res) {
DSSERR("can't get IORESOURCE_MEM HDMI\n");
return -EINVAL;
}
/* Base address taken from platform */
- hdmi.ip_data.base_wp = ioremap(hdmi_mem->start,
- resource_size(hdmi_mem));
+ hdmi.ip_data.base_wp = ioremap(res->start, resource_size(res));
if (!hdmi.ip_data.base_wp) {
DSSERR("can't ioremap WP\n");
return -ENOMEM;
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/7] OMAPDSS: HDMI: Convert to devm_request_and_ioremap
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 1/7] OMAPDSS: HDMI: Rename resource variable at probe Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 3/7] OMAPDSS: HDMI: Make panel return dssdev register errors Ricardo Neri
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
Using devm_request_and_ioremap provides better memory handling and
improves readability.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/hdmi.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index cdb043d..51ee0a6 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -1012,7 +1012,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
}
/* Base address taken from platform */
- hdmi.ip_data.base_wp = ioremap(res->start, resource_size(res));
+ hdmi.ip_data.base_wp = devm_request_and_ioremap(&pdev->dev, res);
if (!hdmi.ip_data.base_wp) {
DSSERR("can't ioremap WP\n");
return -ENOMEM;
@@ -1020,7 +1020,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
r = hdmi_get_clocks(pdev);
if (r) {
- iounmap(hdmi.ip_data.base_wp);
+ DSSERR("can't get clocks\n");
return r;
}
@@ -1065,8 +1065,6 @@ static int __exit omapdss_hdmihw_remove(struct platform_device *pdev)
hdmi_put_clocks();
- iounmap(hdmi.ip_data.base_wp);
-
return 0;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/7] OMAPDSS: HDMI: Make panel return dssdev register errors
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 1/7] OMAPDSS: HDMI: Rename resource variable at probe Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 2/7] OMAPDSS: HDMI: Convert to devm_request_and_ioremap Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 4/7] OMAPDSS: HDMI: Handle panel init error at probe Ricardo Neri
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
Do not assume blindly that the DSS driver was registered successfully.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/hdmi_panel.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi_panel.c b/drivers/video/omap2/dss/hdmi_panel.c
index 69fb115..129107b 100644
--- a/drivers/video/omap2/dss/hdmi_panel.c
+++ b/drivers/video/omap2/dss/hdmi_panel.c
@@ -454,9 +454,7 @@ int hdmi_panel_init(void)
spin_lock_init(&hdmi.audio_lock);
#endif
- omap_dss_register_driver(&hdmi_driver);
-
- return 0;
+ return omap_dss_register_driver(&hdmi_driver);
}
void hdmi_panel_exit(void)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/7] OMAPDSS: HDMI: Handle panel init error at probe
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
` (2 preceding siblings ...)
2012-11-03 0:31 ` [PATCH v2 3/7] OMAPDSS: HDMI: Make panel return dssdev register errors Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 5/7] OMAPDSS: HDMI: Uninit display on device add error Ricardo Neri
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
Do not blindly assume that the panel could be initialized.
While there, group mutex initialization at a single place.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/hdmi.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 51ee0a6..696386c 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -60,6 +60,7 @@
static struct {
struct mutex lock;
struct platform_device *pdev;
+
struct hdmi_ip_data ip_data;
struct clk *sys_clk;
@@ -1004,6 +1005,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi.pdev = pdev;
mutex_init(&hdmi.lock);
+ mutex_init(&hdmi.ip_data.lock);
res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -1031,9 +1033,11 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
hdmi.ip_data.phy_offset = HDMI_PHY;
- mutex_init(&hdmi.ip_data.lock);
-
- hdmi_panel_init();
+ r = hdmi_panel_init();
+ if (r) {
+ DSSERR("can't init panel\n");
+ goto err_panel_init;
+ }
dss_debugfs_create_file("hdmi", hdmi_dump_regs);
@@ -1042,6 +1046,10 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi_probe_pdata(pdev);
return 0;
+
+err_panel_init:
+ hdmi_put_clocks();
+ return r;
}
static int __exit hdmi_remove_child(struct device *dev, void *data)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/7] OMAPDSS: HDMI: Uninit display on device add error
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
` (3 preceding siblings ...)
2012-11-03 0:31 ` [PATCH v2 4/7] OMAPDSS: HDMI: Handle panel init error at probe Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 6/7] OMAPDSS: HDMI: Add op to get audio DMA port address offset Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 7/7] OMAPDSS: HDMI: Create platform device for audio support Ricardo Neri
6 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
The display must be uninitialized in order to free the requested GPIOs.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/hdmi.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 696386c..4adf830 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -973,6 +973,7 @@ static void __init hdmi_probe_pdata(struct platform_device *pdev)
r = dss_add_device(dssdev);
if (r) {
DSSERR("device %s register failed: %d\n", dssdev->name, r);
+ hdmi_uninit_display(dssdev);
dss_put_device(dssdev);
return;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 6/7] OMAPDSS: HDMI: Add op to get audio DMA port address offset
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
` (4 preceding siblings ...)
2012-11-03 0:31 ` [PATCH v2 5/7] OMAPDSS: HDMI: Uninit display on device add error Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-03 0:31 ` [PATCH v2 7/7] OMAPDSS: HDMI: Create platform device for audio support Ricardo Neri
6 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
It could be possible that the DMA port differs accross diferent HDMI IPs. Thus,
add an IP-specific function to obtain the address offset and size of the DMA
data port.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/dss_features.c | 1 +
drivers/video/omap2/dss/ti_hdmi.h | 3 +++
drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c | 9 +++++++++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 3e8287c..8dcecbc 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -820,6 +820,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
.audio_start = ti_hdmi_4xxx_audio_start,
.audio_stop = ti_hdmi_4xxx_audio_stop,
.audio_config = ti_hdmi_4xxx_audio_config,
+ .audio_get_dma_port = ti_hdmi_4xxx_audio_get_dma_port,
#endif
};
diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
index b046c20..216aa70 100644
--- a/drivers/video/omap2/dss/ti_hdmi.h
+++ b/drivers/video/omap2/dss/ti_hdmi.h
@@ -102,6 +102,8 @@ struct ti_hdmi_ip_ops {
int (*audio_config)(struct hdmi_ip_data *ip_data,
struct omap_dss_audio *audio);
+
+ int (*audio_get_dma_port)(u32 *offset, u32 *size);
#endif
};
@@ -183,5 +185,6 @@ int ti_hdmi_4xxx_audio_start(struct hdmi_ip_data *ip_data);
void ti_hdmi_4xxx_audio_stop(struct hdmi_ip_data *ip_data);
int ti_hdmi_4xxx_audio_config(struct hdmi_ip_data *ip_data,
struct omap_dss_audio *audio);
+int ti_hdmi_4xxx_audio_get_dma_port(u32 *offset, u32 *size);
#endif
#endif
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index a6efff2..e18b222 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -1418,4 +1418,13 @@ void ti_hdmi_4xxx_audio_stop(struct hdmi_ip_data *ip_data)
REG_FLD_MOD(hdmi_wp_base(ip_data),
HDMI_WP_AUDIO_CTRL, false, 30, 30);
}
+
+int ti_hdmi_4xxx_audio_get_dma_port(u32 *offset, u32 *size)
+{
+ if (!offset || !size)
+ return -EINVAL;
+ *offset = HDMI_WP_AUDIO_DATA;
+ *size = 4;
+ return 0;
+}
#endif
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 7/7] OMAPDSS: HDMI: Create platform device for audio support
2012-11-03 0:31 [PATCH v2 0/7] Create platform device for audio support Ricardo Neri
` (5 preceding siblings ...)
2012-11-03 0:31 ` [PATCH v2 6/7] OMAPDSS: HDMI: Add op to get audio DMA port address offset Ricardo Neri
@ 2012-11-03 0:31 ` Ricardo Neri
2012-11-05 8:46 ` Tomi Valkeinen
6 siblings, 1 reply; 10+ messages in thread
From: Ricardo Neri @ 2012-11-03 0:31 UTC (permalink / raw)
To: tomi.valkeinen
Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap, Ricardo Neri
Creating the accessory devices, such as audio, from the HDMI driver
allows to regard HDMI as a single entity with audio an display
functionality. This intends to follow the design of drivers such
as MFD, in which a single entity handles the creation of the accessory
devices. Such devices are then used by domain-specific drivers; audio in
this case.
Also, this is in line with the DT implementation of HDMI, in which we will
have a single node to describe this feature of the OMAP SoC.
Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
---
drivers/video/omap2/dss/hdmi.c | 62 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 4adf830..d6ce4c6 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -60,6 +60,9 @@
static struct {
struct mutex lock;
struct platform_device *pdev;
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+ struct platform_device *audio_pdev;
+#endif
struct hdmi_ip_data ip_data;
@@ -766,6 +769,54 @@ static void hdmi_put_clocks(void)
}
#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+static int hdmi_probe_audio(struct platform_device *pdev)
+{
+ struct resource *res;
+ u32 port_offset, port_size;
+ struct resource aud_res[2] = {
+ DEFINE_RES_MEM(-1, -1),
+ DEFINE_RES_DMA(-1),
+ };
+
+ hdmi.audio_pdev = ERR_PTR(-EINVAL);
+
+ res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ DSSERR("can't get IORESOURCE_MEM HDMI\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Pass DMA audio port to audio drivers.
+ * Audio drivers should not ioremap it.
+ */
+ hdmi.ip_data.ops->audio_get_dma_port(&port_offset, &port_size);
+
+ aud_res[0].start = res->start + port_offset;
+ aud_res[0].end = aud_res[0].start + port_size - 1;
+
+ res = platform_get_resource(hdmi.pdev, IORESOURCE_DMA, 0);
+ if (!res) {
+ DSSERR("can't get IORESOURCE_DMA HDMI\n");
+ return -EINVAL;
+ }
+
+ /* Pass the audio DMA request resource to audio drivers. */
+ aud_res[1].start = res->start;
+
+ /* create platform device for HDMI audio driver */
+ hdmi.audio_pdev = platform_device_register_simple(
+ "omap_hdmi_audio",
+ -1, aud_res,
+ ARRAY_SIZE(aud_res));
+ if (IS_ERR(hdmi.audio_pdev)) {
+ DSSERR("Can't instantiate hdmi-audio\n");
+ return PTR_ERR(hdmi.audio_pdev);
+ }
+
+ return 0;
+}
+
int hdmi_compute_acr(u32 sample_freq, u32 *n, u32 *cts)
{
u32 deep_color;
@@ -1046,6 +1097,12 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi_probe_pdata(pdev);
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+ r = hdmi_probe_audio(pdev);
+ if (r)
+ DSSWARN("could not create platform device for audio");
+#endif
+
return 0;
err_panel_init:
@@ -1062,6 +1119,11 @@ static int __exit hdmi_remove_child(struct device *dev, void *data)
static int __exit omapdss_hdmihw_remove(struct platform_device *pdev)
{
+#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
+ if (!IS_ERR(hdmi.audio_pdev))
+ platform_device_unregister(hdmi.audio_pdev);
+#endif
+
device_for_each_child(&pdev->dev, NULL, hdmi_remove_child);
dss_unregister_child_devices(&pdev->dev);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 7/7] OMAPDSS: HDMI: Create platform device for audio support
2012-11-03 0:31 ` [PATCH v2 7/7] OMAPDSS: HDMI: Create platform device for audio support Ricardo Neri
@ 2012-11-05 8:46 ` Tomi Valkeinen
2012-11-06 5:27 ` Ricardo Neri
0 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2012-11-05 8:46 UTC (permalink / raw)
To: Ricardo Neri; +Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap
[-- Attachment #1: Type: text/plain, Size: 3108 bytes --]
On 2012-11-03 02:31, Ricardo Neri wrote:
> Creating the accessory devices, such as audio, from the HDMI driver
> allows to regard HDMI as a single entity with audio an display
> functionality. This intends to follow the design of drivers such
> as MFD, in which a single entity handles the creation of the accessory
> devices. Such devices are then used by domain-specific drivers; audio in
> this case.
>
> Also, this is in line with the DT implementation of HDMI, in which we will
> have a single node to describe this feature of the OMAP SoC.
>
> Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
> ---
> drivers/video/omap2/dss/hdmi.c | 62 ++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 62 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 4adf830..d6ce4c6 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -60,6 +60,9 @@
> static struct {
> struct mutex lock;
> struct platform_device *pdev;
> +#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
> + struct platform_device *audio_pdev;
> +#endif
>
> struct hdmi_ip_data ip_data;
>
> @@ -766,6 +769,54 @@ static void hdmi_put_clocks(void)
> }
>
> #if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
> +static int hdmi_probe_audio(struct platform_device *pdev)
> +{
> + struct resource *res;
> + u32 port_offset, port_size;
> + struct resource aud_res[2] = {
> + DEFINE_RES_MEM(-1, -1),
> + DEFINE_RES_DMA(-1),
> + };
> +
> + hdmi.audio_pdev = ERR_PTR(-EINVAL);
I don't like this. I think ERR_PTR stuff should be used only for return
values, not when storing pointers. I think it's much more natural and
less error prone to do if (hdmi.audio_pdev == NULL) than if
(IS_ERR(hdmi.audio_pdev)).
So store the return value from platform_dev_register first to a local
variable, check IS_ERR for that, and only then store it to hdmi.audio_pdev.
> + res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + DSSERR("can't get IORESOURCE_MEM HDMI\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Pass DMA audio port to audio drivers.
> + * Audio drivers should not ioremap it.
> + */
> + hdmi.ip_data.ops->audio_get_dma_port(&port_offset, &port_size);
> +
> + aud_res[0].start = res->start + port_offset;
> + aud_res[0].end = aud_res[0].start + port_size - 1;
> +
> + res = platform_get_resource(hdmi.pdev, IORESOURCE_DMA, 0);
> + if (!res) {
> + DSSERR("can't get IORESOURCE_DMA HDMI\n");
> + return -EINVAL;
> + }
> +
> + /* Pass the audio DMA request resource to audio drivers. */
> + aud_res[1].start = res->start;
> +
> + /* create platform device for HDMI audio driver */
> + hdmi.audio_pdev = platform_device_register_simple(
> + "omap_hdmi_audio",
> + -1, aud_res,
> + ARRAY_SIZE(aud_res));
Not a problem for the time being, but the above prevents us from having
two HDMI outputs. Perhaps you could use hdmi pdev's ID instead of -1 above?
Otherwise I think the series is ok.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 7/7] OMAPDSS: HDMI: Create platform device for audio support
2012-11-05 8:46 ` Tomi Valkeinen
@ 2012-11-06 5:27 ` Ricardo Neri
0 siblings, 0 replies; 10+ messages in thread
From: Ricardo Neri @ 2012-11-06 5:27 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: peter.ujfalusi, s-guiriec, b-cousson, linux-omap
Hi Tomi,
Thanks for reviewing.
On 11/05/2012 02:46 AM, Tomi Valkeinen wrote:
> On 2012-11-03 02:31, Ricardo Neri wrote:
>> Creating the accessory devices, such as audio, from the HDMI driver
>> allows to regard HDMI as a single entity with audio an display
>> functionality. This intends to follow the design of drivers such
>> as MFD, in which a single entity handles the creation of the accessory
>> devices. Such devices are then used by domain-specific drivers; audio in
>> this case.
>>
>> Also, this is in line with the DT implementation of HDMI, in which we will
>> have a single node to describe this feature of the OMAP SoC.
>>
>> Signed-off-by: Ricardo Neri <ricardo.neri@ti.com>
>> ---
>> drivers/video/omap2/dss/hdmi.c | 62 ++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 62 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
>> index 4adf830..d6ce4c6 100644
>> --- a/drivers/video/omap2/dss/hdmi.c
>> +++ b/drivers/video/omap2/dss/hdmi.c
>> @@ -60,6 +60,9 @@
>> static struct {
>> struct mutex lock;
>> struct platform_device *pdev;
>> +#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
>> + struct platform_device *audio_pdev;
>> +#endif
>>
>> struct hdmi_ip_data ip_data;
>>
>> @@ -766,6 +769,54 @@ static void hdmi_put_clocks(void)
>> }
>>
>> #if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
>> +static int hdmi_probe_audio(struct platform_device *pdev)
>> +{
>> + struct resource *res;
>> + u32 port_offset, port_size;
>> + struct resource aud_res[2] = {
>> + DEFINE_RES_MEM(-1, -1),
>> + DEFINE_RES_DMA(-1),
>> + };
>> +
>> + hdmi.audio_pdev = ERR_PTR(-EINVAL);
>
> I don't like this. I think ERR_PTR stuff should be used only for return
> values, not when storing pointers. I think it's much more natural and
> less error prone to do if (hdmi.audio_pdev == NULL) than if
> (IS_ERR(hdmi.audio_pdev)).
>
> So store the return value from platform_dev_register first to a local
> variable, check IS_ERR for that, and only then store it to hdmi.audio_pdev.
OK. I'll implement the change.
>
>> + res = platform_get_resource(hdmi.pdev, IORESOURCE_MEM, 0);
>> + if (!res) {
>> + DSSERR("can't get IORESOURCE_MEM HDMI\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * Pass DMA audio port to audio drivers.
>> + * Audio drivers should not ioremap it.
>> + */
>> + hdmi.ip_data.ops->audio_get_dma_port(&port_offset, &port_size);
>> +
>> + aud_res[0].start = res->start + port_offset;
>> + aud_res[0].end = aud_res[0].start + port_size - 1;
>> +
>> + res = platform_get_resource(hdmi.pdev, IORESOURCE_DMA, 0);
>> + if (!res) {
>> + DSSERR("can't get IORESOURCE_DMA HDMI\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Pass the audio DMA request resource to audio drivers. */
>> + aud_res[1].start = res->start;
>> +
>> + /* create platform device for HDMI audio driver */
>> + hdmi.audio_pdev = platform_device_register_simple(
>> + "omap_hdmi_audio",
>> + -1, aud_res,
>> + ARRAY_SIZE(aud_res));
>
> Not a problem for the time being, but the above prevents us from having
> two HDMI outputs. Perhaps you could use hdmi pdev's ID instead of -1 above?
I guess it is fine as you suggest. I will have to work on the ASoC side
to support more than one HDMI outputs as well.
BR,
Ricardo
>
> Otherwise I think the series is ok.
>
> Tomi
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread