Re: Expected behavior for S_INPUT while streaming in progress?
Hi Devin, On 07/22/2013 09:48 PM, Devin Heitmueller wrote: Hello, I'm doing some cleanup on the au8522 driver, and one thing I noticed was that you are actually allowed to issue an S_INPUT while streaming is active. This seems like dangerous behavior, since it can result in things like the standard and/or resolution changing while the device is actively streaming video. Should we be returning -EBUSY for S_INPUT if streaming is in progress? I see cases in drivers where we prevent S_STD from working while streaming is in progress, but it seems like S_INPUT is a superset of S_STD (it typically happens earlier in the setup process). Some drivers already return EBUSY for S_INPUT (ivtv). If the hardware can't safely support it to switch inputs while streaming, then there is no option but to return EBUSY. Changing from one SDTV input to another SDTV input should not trigger a change of standard, so that in itself should not be a cause for blocking S_INPUT while streaming. Switching between e.g. a HDTV and a SDTV input is a different matter: there the resolution really changes and that should return an error while streaming. If we did do this, how badly do we think it would break existing applications? It could require applications to do a STREAMOFF before setting the input (to handle the possibility that the call wasn't issued previously when an app was done with the device), which I suspect many applications aren't doing today. Most drivers allow switching between e.g. the tuner and composite input on the fly while streaming. Some don't like ivtv where the hardware can't resync properly. I know there where problems with applications at the time when I made that change in ivtv, but I haven't heard of it in a long time, so applications may be fixed. Alternatively, we can based it on not just whether streamon was called and instead base it on the combination of streamon having been called and a filehandle actively doing streaming. In this case case would return EBUSY if both conditions were met, but if only streamon was called but streaming wasn't actively being done by a filehandle, we would internally call streamoff and then change the input. This would mean that if an application like tvtime were running, externally toggling the input would return EBUSY. But if nothing was actively streaming via a /dev/videoX device then the call to set input would be successful (after internally stopping the stream). This seems overkill. When dealing with SDTV inputs I would just make sure that switching inputs will not change the standard, which is a perfectly reasonable thing to do. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi Alan,
On Monday 22 of July 2013 10:44:39 Alan Stern wrote:
On Mon, 22 Jul 2013, Kishon Vijay Abraham I wrote:
The PHY and the controller it is attached to are both physical
devices.
The connection between them is hardwired by the system
manufacturer and cannot be changed by software.
PHYs are generally described by fixed system-specific board
files or by Device Tree information. Are they ever discovered
dynamically?
No. They are created just like any other platform devices are created.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
Is the same true for the controllers attached to the PHYs?
If not -- if both a PHY and a controller are discovered
dynamically -- how does the kernel know whether they are
connected to each other?
No differences here. Both PHY and controller will have dt information
or hwmod data using which platform devices will be created.
The kernel needs to know which controller is attached to which
PHY. Currently this information is represented by name or ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous replies.
Without any extra data in platform data. (I have even posted a code
example.)
The PHY's driver (the supplier) uses the platform data to
construct a platform_device structure that represents the PHY.
Currently the driver assigns static labels (corresponding to the label
used in the platform data of the controller).
Until this is done, the controller's driver (the client) cannot
use the PHY.
right.
Since there is no parent-child relation between the PHY and the
controller, there is no guarantee that the PHY's driver will be
ready when the controller's driver wants to use it. A deferred
probe may be needed.
right.
The issue (or one of the issues) in this discussion is that
Greg does not like the idea of using names or IDs to associate
PHYs with controllers, because they are too prone to
duplications or other errors. Pointers are more reliable.
But pointers to what? Since the only data known to be
available to both the PHY driver and controller driver is the
platform data, the obvious answer is a pointer to platform data
(either for the PHY or for the controller, or maybe both).
hmm.. it's not going to be simple though as the platform device for
the PHY and controller can be created in entirely different places.
e.g., in some cases the PHY device is a child of some mfd core device
(the device will be created in drivers/mfd) and the controller driver
(usually) is created in board file. I guess then we have to come up
with something to share a pointer in two different files.
The ability for two different source files to share a pointer to a data
item defined in a third source file has been around since long before
the C language was invented. :-)
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host drivers.
This means you would have to export symbol with the same name from both
PHY drivers, which obviously would not work in this case, because having
both drivers enabled (in a multiplatform aware configuration) would lead
to linking conflict.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
[Fixed address of devicetree mailing list and added more people on CC.]
For reference, full thread can be found under following link:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813
Best regards,
Tomasz
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
On Monday 22 of July 2013 10:44:39 Alan Stern wrote:
On Mon, 22 Jul 2013, Kishon Vijay Abraham I wrote:
The PHY and the controller it is attached to are both
physical
devices.
The connection between them is hardwired by the system
manufacturer and cannot be changed by software.
PHYs are generally described by fixed system-specific
board
files or by Device Tree information. Are they ever
discovered
dynamically?
No. They are created just like any other platform devices are
created.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
Is the same true for the controllers attached to the PHYs?
If not -- if both a PHY and a controller are discovered
dynamically -- how does the kernel know whether they are
connected to each other?
No differences here. Both PHY and controller will have dt
information
or hwmod data using which platform devices will be created.
The kernel needs to know which controller is attached to
which
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
The PHY's driver (the supplier) uses the platform data to
construct a platform_device structure that represents the
PHY.
Currently the driver assigns static labels (corresponding to the
label
used in the platform data of the controller).
Until this is done, the controller's driver (the client)
cannot
use the PHY.
right.
Since there is no parent-child relation between the PHY
and the
controller, there is no guarantee that the PHY's driver
will be
ready when the controller's driver wants to use it. A
deferred
probe may be needed.
right.
The issue (or one of the issues) in this discussion is
that
Greg does not like the idea of using names or IDs to
associate
PHYs with controllers, because they are too prone to
duplications or other errors. Pointers are more reliable.
But pointers to what? Since the only data known to be
available to both the PHY driver and controller driver is
the
platform data, the obvious answer is a pointer to platform
data
(either for the PHY or for the controller, or maybe both).
hmm.. it's not going to be simple though as the platform device for
the PHY and controller can be created in entirely different places.
e.g., in some cases the PHY device is a child of some mfd core
device
(the device will be created in drivers/mfd) and the controller
driver
(usually) is created in board file. I guess then we have to come up
with something to share a pointer in two different files.
The ability for two different source files to share a pointer to a
data
item defined in a third source file has been around since long before
the C language was invented. :-)
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
Re: [PATCH 0/5] Davinci VPBE use devres and some cleanup
Hi Hans, On Sat, Jul 13, 2013 at 2:20 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series replaces existing resource handling in the driver with managed device resource. Lad, Prabhakar (5): media: davinci: vpbe_venc: convert to devm_* api media: davinci: vpbe_osd: convert to devm_* api media: davinci: vpbe_display: convert to devm* api media: davinci: vpss: convert to devm* api can you pick up patches 1-4 for 3.12 ? I'll handle the 5/5 patch later. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/5] Davinci VPBE use devres and some cleanup
All, I do not know why I keep receiving these e-mails from multiple people. Could you please remove me from your e-mail lists? Thanks! -Tobin Abraham -Original Message- From: davinci-linux-open-source-bounces+t-abraham=ti@linux.davincidsp.com [mailto:davinci-linux-open-source-bounces+t-abraham=ti@linux.davincidsp.com] On Behalf Of Prabhakar Lad Sent: Tuesday, July 23, 2013 6:18 AM To: Hans Verkuil Cc: DLOS; LKML; LMML Subject: Re: [PATCH 0/5] Davinci VPBE use devres and some cleanup Hi Hans, On Sat, Jul 13, 2013 at 2:20 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series replaces existing resource handling in the driver with managed device resource. Lad, Prabhakar (5): media: davinci: vpbe_venc: convert to devm_* api media: davinci: vpbe_osd: convert to devm_* api media: davinci: vpbe_display: convert to devm* api media: davinci: vpss: convert to devm* api can you pick up patches 1-4 for 3.12 ? I'll handle the 5/5 patch later. Regards, --Prabhakar Lad ___ Davinci-linux-open-source mailing list davinci-linux-open-sou...@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Davinci VPBE use devres and some cleanup
On Tue 23 July 2013 13:17:43 Prabhakar Lad wrote: Hi Hans, On Sat, Jul 13, 2013 at 2:20 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series replaces existing resource handling in the driver with managed device resource. Lad, Prabhakar (5): media: davinci: vpbe_venc: convert to devm_* api media: davinci: vpbe_osd: convert to devm_* api media: davinci: vpbe_display: convert to devm* api media: davinci: vpss: convert to devm* api can you pick up patches 1-4 for 3.12 ? I'll handle the 5/5 patch later. Will do. I'm planning on a new pull request during/around the weekend. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Davinci VPBE use devres and some cleanup
Abraham, On Tue, Jul 23, 2013 at 5:14 PM, Abraham, Tobin t-abra...@ti.com wrote: All, I do not know why I keep receiving these e-mails from multiple people. Could you please remove me from your e-mail lists? Please follow this link [1] to un subscribe yourself from the mailing list. [1] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-usb-v2: fix Kconfig dependency when RC_CORE=m
It is not allowed to build driver as a build-in when RC_CORE (remote controller support) is build as a module. randconfig build error with next-20130719, in drivers/media/usb drivers/built-in.o: In function `dvb_usbv2_disconnect': (.text+0x154b39): undefined reference to `rc_unregister_device' drivers/built-in.o: In function `dvb_usbv2_init_work': dvb_usb_core.c:(.text+0x155b2d): undefined reference to `rc_allocate_device' dvb_usb_core.c:(.text+0x155c38): undefined reference to `rc_register_device' dvb_usb_core.c:(.text+0x155c5b): undefined reference to `rc_free_device' drivers/built-in.o: In function `anysee_rc_query': anysee.c:(.text+0x157795): undefined reference to `rc_keydown' Reported-by: Jim Davis jim.ep...@gmail.com Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/usb/dvb-usb-v2/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/Kconfig b/drivers/media/usb/dvb-usb-v2/Kconfig index a3c8ecf..2059d0c 100644 --- a/drivers/media/usb/dvb-usb-v2/Kconfig +++ b/drivers/media/usb/dvb-usb-v2/Kconfig @@ -1,6 +1,6 @@ config DVB_USB_V2 tristate Support for various USB DVB devices v2 - depends on DVB_CORE USB I2C + depends on DVB_CORE USB I2C (RC_CORE || RC_CORE=n) help By enabling this you will be able to choose the various supported USB1.1 and USB2.0 DVB devices. -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] dvb_usb_v2: fix Kconfig dependency when RC_CORE=m
The following changes since commit 1c26190a8d492adadac4711fe5762d46204b18b0: [media] exynos4-is: Correct colorspace handling at FIMC-LITE (2013-06-28 15:33:27 -0300) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git kconfig_fix for you to fetch changes up to 93d3452e72b2b5db15161a9f2d99949e55428caf: dvb-usb-v2: fix Kconfig dependency when RC_CORE=m (2013-07-23 15:15:31 +0300) Antti Palosaari (1): dvb-usb-v2: fix Kconfig dependency when RC_CORE=m drivers/media/usb/dvb-usb-v2/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gp8psk: tuning changes
Em Mon, 22 Jul 2013 20:24:41 -0600
Chris Lee update...@gmail.com escreveu:
- cleanup tuning code
- fix tuning for some systems/modulations/fecs
- add Digicipher II and DSS tuning abilities
- update the property_cache once tuning succeeds with the actual tuned values
- implement gp8psk_fe_read_ber()
- update .delsys with the new systems
Its a pretty big patch, I can reduce it to smaller parts if you want,
I welcome comments questions
Yes, please split the API changes on a separate patch, and add the
corresponding changes at Documentation/DocBook/media/dvb to reflect
the needed API additions for Digicipher II and DSS.
Then, you can add one patch per logical change at the gp8psk driver.
git citool is your friend: using it, you can easily break the patch
into smaller ones.
Just keep in mind that the driver should keep compiling after each patch,
and try to maintain the one logical change per patch. Don't be shy
on the comments: we love comments on the patch descriptions, as they
can help a lot developers when they need to discover why the driver
stopped working, or what should be done on a similar driver to address
a common issue.
Btw, your emailer completely broke this patch: it broke long lines,
it replaced tabs by spaces, etc.
You should either use a different emailer, or to use git send-email
to send the patches directly to your smtp server.
Regards,
Mauro
Thanks
Signed-off-by: Chris Lee update...@gmail.com
--- media_tree/include/uapi/linux/dvb/frontend.h 2013-07-18
11:19:29.601746158 -0600
+++ media_tree.test/include/uapi/linux/dvb/frontend.h 2013-07-22
20:10:50.658719256 -0600
@@ -165,6 +165,7 @@
FEC_3_5,
FEC_9_10,
FEC_2_5,
+ FEC_5_11,
} fe_code_rate_t;
@@ -410,6 +411,10 @@
SYS_DVBT2,
SYS_TURBO,
SYS_DVBC_ANNEX_C,
+ SYS_DCII_C_QPSK,
+ SYS_DCII_I_QPSK,
+ SYS_DCII_Q_QPSK,
+ SYS_DCII_C_OQPSK,
} fe_delivery_system_t;
/* backward compatibility */
--- media_tree/drivers/media/usb/dvb-usb/gp8psk.h 2013-07-18
11:19:28.261746125 -0600
+++ media_tree.test/drivers/media/usb/dvb-usb/gp8psk.h 2013-07-22
19:54:19.642694697 -0600
@@ -52,6 +52,8 @@
#define GET_SERIAL_NUMBER 0x93/* in */
#define USE_EXTRA_VOLT 0x94
#define GET_FPGA_VERS 0x95
+#define GET_SIGNAL_STAT 0x9A
+#define GET_BER_RATE 0x9B
#define CW3K_INIT 0x9d
/* PSK_configuration bits */
--- media_tree/drivers/media/usb/dvb-usb/gp8psk-fe.c 2013-07-18
11:19:28.261746125 -0600
+++ media_tree.test/drivers/media/usb/dvb-usb/gp8psk-fe.c 2013-07-22
20:10:20.582718511 -0600
@@ -45,7 +45,8 @@
if (time_after(jiffies,st-next_status_check)) {
gp8psk_usb_in_op(st-d, GET_SIGNAL_LOCK, 0,0,st-lock,1);
gp8psk_usb_in_op(st-d, GET_SIGNAL_STRENGTH, 0,0,buf,6);
- st-snr = (buf[1]) 8 | buf[0];
+
+ st-snr = ((buf[1]) 8 | buf[0]) 4;
st-next_status_check = jiffies + (st-status_check_interval*HZ)/1000;
}
return 0;
@@ -53,7 +54,42 @@
static int gp8psk_fe_read_status(struct dvb_frontend* fe, fe_status_t
*status)
{
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
struct gp8psk_fe_state *st = fe-demodulator_priv;
+
+ u8 buf[32];
+ int frequency;
+ int carrier_error;
+ int carrier_offset;
+ int rate_error;
+ int rate_offset;
+ int symbol_rate;
+
+ int fe_gp8psk_system_return[] = {
+ SYS_DVBS,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_DCII_C_QPSK,
+ SYS_DCII_I_QPSK,
+ SYS_DCII_Q_QPSK,
+ SYS_DCII_C_OQPSK,
+ SYS_DSS,
+ SYS_UNDEFINED
+ };
+
+ int fe_gp8psk_modulation_return[] = {
+ QPSK,
+ QPSK,
+ PSK_8,
+ QAM_16,
+ QPSK,
+ QPSK,
+ QPSK,
+ QPSK,
+ QPSK,
+ };
+
gp8psk_fe_update_status(st);
if (st-lock)
@@ -61,18 +97,97 @@
else
*status = 0;
- if (*status FE_HAS_LOCK)
+ if (*status FE_HAS_LOCK) {
+ gp8psk_usb_in_op(st-d, GET_SIGNAL_STAT, 0, 0, buf, 32);
+ frequency = ((buf[11] 24) + (buf[10] 16) + (buf[9] 8) +
buf[8]) / 1000;
+ carrier_error = ((buf[15] 24) + (buf[14] 16) + (buf[13] 8)
+ buf[12]) / 1000;
+ carrier_offset = (buf[19] 24) + (buf[18] 16) + (buf[17] 8)
+ buf[16];
+ rate_error = (buf[23] 24) + (buf[22] 16) + (buf[21] 8) + buf[20];
+ rate_offset = (buf[27] 24) + (buf[26] 16) + (buf[25] 8) + buf[24];
+ symbol_rate = (buf[31] 24) + (buf[30] 16) + (buf[29] 8) + buf[28];
+
+ c-frequency = frequency - carrier_error;
+ c-symbol_rate = symbol_rate + rate_error;
+
+ switch (c-delivery_system) {
+ case SYS_DSS:
+ case SYS_DVBS:
+ c-delivery_system = fe_gp8psk_system_return[buf[1]];
+ c-modulation = fe_gp8psk_modulation_return[buf[1]];
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_1_2; break;
+ case 1: c-fec_inner = FEC_2_3; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_5_6; break;
+ case 4: c-fec_inner = FEC_6_7; break;
+ case 5: c-fec_inner = FEC_7_8; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ break;
+ case SYS_TURBO:
+ c-delivery_system =
Re: [PATCH v3 1/3] [media] coda: Fix error paths
Hi Fabio,
Am Montag, den 22.07.2013, 22:38 -0300 schrieb Fabio Estevam:
From: Fabio Estevam fabio.este...@freescale.com
Some resources were not being released in the error path and some were
released
in the incorrect order.
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Changes since v2:
- Newly introduced in this series
drivers/media/platform/coda.c | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index df4ada88..ea16c20 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -1514,15 +1514,17 @@ static int coda_open(struct file *file)
int ret = 0;
int idx;
- idx = coda_next_free_instance(dev);
- if (idx = CODA_MAX_INSTANCES)
- return -EBUSY;
- set_bit(idx, dev-instance_mask);
-
ctx = kzalloc(sizeof *ctx, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
+ idx = coda_next_free_instance(dev);
+ if (idx = CODA_MAX_INSTANCES) {
+ ret = -EBUSY;
+ goto err_coda_max;
+ }
+ set_bit(idx, dev-instance_mask);
+
v4l2_fh_init(ctx-fh, video_devdata(file));
file-private_data = ctx-fh;
v4l2_fh_add(ctx-fh);
@@ -1537,12 +1539,12 @@ static int coda_open(struct file *file)
v4l2_err(dev-v4l2_dev, %s return error (%d)\n,
__func__, ret);
- goto err;
+ goto err_ctx_init;
}
ret = coda_ctrls_setup(ctx);
if (ret) {
v4l2_err(dev-v4l2_dev, failed to setup coda controls\n);
- goto err;
+ goto err_ctrls_setup;
}
ctx-fh.ctrl_handler = ctx-ctrls;
@@ -1552,7 +1554,7 @@ static int coda_open(struct file *file)
if (!ctx-parabuf.vaddr) {
v4l2_err(dev-v4l2_dev, failed to allocate parabuf);
ret = -ENOMEM;
- goto err;
+ goto err_dma_alloc;
}
coda_lock(ctx);
@@ -1567,9 +1569,15 @@ static int coda_open(struct file *file)
return 0;
-err:
+err_dma_alloc:
+ v4l2_ctrl_handler_free(ctx-ctrls);
+err_ctrls_setup:
+ v4l2_m2m_ctx_release(ctx-m2m_ctx);
+err_ctx_init:
v4l2_fh_del(ctx-fh);
v4l2_fh_exit(ctx-fh);
+ clear_bit(ctx-idx, dev-instance_mask);
+err_coda_max:
kfree(ctx);
return ret;
}
@@ -1582,16 +1590,16 @@ static int coda_release(struct file *file)
v4l2_dbg(1, coda_debug, dev-v4l2_dev, Releasing instance %p\n,
ctx);
+ clk_disable_unprepare(dev-clk_ahb);
+ clk_disable_unprepare(dev-clk_per);
coda_lock(ctx);
list_del(ctx-list);
coda_unlock(ctx);
dma_free_coherent(dev-plat_dev-dev, CODA_PARA_BUF_SIZE,
ctx-parabuf.vaddr, ctx-parabuf.paddr);
- v4l2_m2m_ctx_release(ctx-m2m_ctx);
v4l2_ctrl_handler_free(ctx-ctrls);
- clk_disable_unprepare(dev-clk_per);
- clk_disable_unprepare(dev-clk_ahb);
+ v4l2_m2m_ctx_release(ctx-m2m_ctx);
the clocks must not be disabled until after v4l2_m2m_ctx_release
returns: this function might wait for the currently running job to
finish.
v4l2_fh_del(ctx-fh);
v4l2_fh_exit(ctx-fh);
clear_bit(ctx-idx, dev-instance_mask);
regards
Philipp
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
Em Mon, 22 Jul 2013 19:28:55 -0600
Chris Lee update...@gmail.com escreveu:
By using DTV_SET_PROPERTY I can run though a list of possible systems
to determine what is supported and what isnt. I havent looked too far
but I think it uses delsys to determine this information. Which I can
already get from DTV_ENUM_DELSYS. This functionality could be expanded
to delmod and delfec.
But there is no way to pol modulations or fec using DTV_SET_PROPERTY.
I understand there is FE_CAN_*** for modulations and fecs but its far
from complete and not very expandable. If we only went on FE_CAN_ for
fec we'd be missing about half the supported fec's.
Ultimately Id like a solution that would have modulations per system,
and fec per modulation as they are quite often different. The
delsys/delmod/delfec is a simpler cleaner interface that is adding new
functionality and wouldnt be required for drivers or userland to
implement if they dont want to as the patch stands (if we implemented
DTV_SET_PROPERTY checks against delmod/delfec then it would be
required in drivers)
So Id love to hear more comments on this subject, it would really be
nice to clean some of the inadequacies up, imo userland should have
the ability to pol the driver for supported systems/modulation/fec vs
just trying everything and seeing what works and what doesnt.
Well, if we're going to properly implement it, then we need to deprecate
the .caps field at the dvb structure, replacing it by the new DVBv5
equivalent, adding a DVBv3 backward code at dvb_frontend.c that will use
the new DVBv5 caps to fill the DVBv3 caps.
Thanks,
Chris
On Mon, Jul 22, 2013 at 5:21 AM, Mauro Carvalho Chehab
m.che...@samsung.com wrote:
Hi Chris,
Em Fri, 19 Jul 2013 14:27:09 -0600
Chris Lee update...@gmail.com escreveu:
In frontend.h we have a struct called dvb_frontend_ops, in there we
have an element called delsys to show the delivery systems supported
by the tuner, Id like to propose we add onto that with delmod and
delfec.
Its not a perfect solution as sometimes a specific modulation or fec
is only availible on specific systems. But its better then what we
have now. The struct fe_caps isnt really suited for this, its missing
many systems, modulations, and fec's. Its just not expandable enough
to get all the supported sys/mod/fec a tuner supports in there.
Expanding this would allow user land applications to poll the tuner to
determine more detailed information on the tuners capabilities.
Here is the patch I propose, along with the au8522 driver modified to
utilize the new elements. Id like to hear comments on it. Does anyone
see a better way of doing this ?
We had a discussion some time ago about it. Basically, a device that
it is said to support, let's say, DVB-T, should support all possible
modulations and FECs that are part of the system.
So, in thesis, there shouldn't be any need to add a list of modulations
and FECs.
Also, frontends that support multiple delivery systems would need
to enumerate the modulations and FECs after the selection of a given
delivery system (as, typically, they only support a subset of them
for each delsys).
Ok, practice is different, as there are reverse-engineered drivers
that may not support everything that the hardware supports. Also,
a few hardware may have additional restrictions.
Yet, on those cases, the userspace may detect if a given modulation
type or FEC is supported, by trying to set it and check if the
operation didn't fail, and if the cache got properly updated.
So, at the end, it was decided to not add anything like that.
Yet, it is good to see other opinions.
It should be said that one of the hard parts of an approach like
that, is that someone would need to dig into each driver and add
the proper support for per-delsys modulation and FECs.
Alternatively, the core could initialize it to the default value
for each standard, and call some driver-specific function that
would reset the modulation/FECs that aren't supported by some
specific drivers.
Regards,
Mauro
Chris Lee update...@gmail.com
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index 1f925e8..f5df08e 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1036,6 +1036,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND +
1] = {
_DTV_CMD(DTV_API_VERSION, 0, 0),
_DTV_CMD(DTV_ENUM_DELSYS, 0, 0),
+ _DTV_CMD(DTV_ENUM_DELMOD, 0, 0),
+ _DTV_CMD(DTV_ENUM_DELFEC, 0, 0),
_DTV_CMD(DTV_ATSCMH_PARADE_ID, 1, 0),
_DTV_CMD(DTV_ATSCMH_RS_FRAME_ENSEMBLE, 1, 0),
@@ -1285,6 +1287,22 @@ static int dtv_property_process_get(struct
dvb_frontend *fe,
}
tvp-u.buffer.len = ncaps;
break;
+ case DTV_ENUM_DELMOD:
+ ncaps = 0;
+ while (fe-ops.delmod[ncaps] ncaps MAX_DELMOD) {
+
Re: [PATCH v3 2/3] [media] coda: Check the return value from clk_prepare_enable()
Hi Fabio, Am Montag, den 22.07.2013, 22:38 -0300 schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com clk_prepare_enable() may fail, so let's check its return value and propagate it in the case of error. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- - Changes since v2: - Release the previously acquired resources Changes since v1: - Add missing 'if' drivers/media/platform/coda.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index ea16c20..5f15aaa 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -1561,14 +1561,27 @@ static int coda_open(struct file *file) list_add(ctx-list, dev-instances); coda_unlock(ctx); - clk_prepare_enable(dev-clk_per); - clk_prepare_enable(dev-clk_ahb); + ret = clk_prepare_enable(dev-clk_per); + if (ret) + goto err_clk_per; + + ret = clk_prepare_enable(dev-clk_ahb); + if (ret) + goto err_clk_ahb; v4l2_dbg(1, coda_debug, dev-v4l2_dev, Created instance %d (%p)\n, ctx-idx, ctx); return 0; +err_clk_ahb: + clk_disable_unprepare(dev-clk_per); +err_clk_per: + coda_lock(ctx); + list_del(ctx-list); + coda_unlock(ctx); + dma_free_coherent(dev-plat_dev-dev, CODA_PARA_BUF_SIZE, + ctx-parabuf.vaddr, ctx-parabuf.paddr); err_dma_alloc: v4l2_ctrl_handler_free(ctx-ctrls); err_ctrls_setup: I still think the list_add() should be moved after the last possible error case and the lock/list_del/unlock should be removed from the error path. regards Philipp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
On Sat, Jul 20, 2013 at 1:57 AM, Chris Lee update...@gmail.com wrote: In frontend.h we have a struct called dvb_frontend_ops, in there we have an element called delsys to show the delivery systems supported by the tuner, Id like to propose we add onto that with delmod and delfec. Its not a perfect solution as sometimes a specific modulation or fec is only availible on specific systems. But its better then what we have now. The struct fe_caps isnt really suited for this, its missing many systems, modulations, and fec's. Its just not expandable enough to get all the supported sys/mod/fec a tuner supports in there. Question Why should an application know all the modulations and FEC's supported by a demodulator ? Aren't demodulators compliant to their respective delivery systems ? Or do you mean to state that, you are trying to work around some demodulator quirks ? Regards, Manu -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
And so on. This explicitly gives the connection between PHYs and
controllers. The PHY providers would use phy1 or phy2, and the PHY
consumers would use host1 or host2.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 10:37:05 Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted
a
code example.)
I don't understand, because I don't know what a PHY lookup does.
I have provided a code example in [1]. Feel free to ask questions about
those code snippets.
[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813/focus=20889
In this case, it doesn't matter where the platform_device
structures
are created or where the driver source code is. Let's take a
simple
example. Suppose the system design includes a PHY named foo.
Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make
up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the
same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation -
you
export random driver-specific symbols from one driver to another.
Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
It's still a random, driver-specific global symbol exported from board file
to drivers.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2
and
there are two types of consumer drivers (e.g. USB host controllers).
Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same
name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
And so on. This explicitly gives the connection between PHYs and
controllers. The PHY providers would use phy1 or phy2, and the PHY
consumers would use host1 or host2.
This could work assuming that only one SoC and one board is supported in
single kernel image. However it's not the case.
We've used to support multiple boards since a long time already and now for
selected platforms we even support multiplatform, i.e. multiple SoCs in
single zImage. Such solution will not work.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gp8psk: tuning changes
Will do, thanks for being patient, Im new to submitting patch's. I've
got lots more coming :)
Digicipher requires no additional changes, Digicipher is close enough
to dvb that you can tune it and use any app you want just like normal.
DSS is another story, I'll leave that patch for last as I can include
the variable packet size patch with it that Im sure will need more
scrutinization. DSS uses 131 byte packets vs 188 byte, so dvb_demux
see's some changes too.
Chris
On Tue, Jul 23, 2013 at 6:55 AM, Mauro Carvalho Chehab
m.che...@samsung.com wrote:
Em Mon, 22 Jul 2013 20:24:41 -0600
Chris Lee update...@gmail.com escreveu:
- cleanup tuning code
- fix tuning for some systems/modulations/fecs
- add Digicipher II and DSS tuning abilities
- update the property_cache once tuning succeeds with the actual tuned values
- implement gp8psk_fe_read_ber()
- update .delsys with the new systems
Its a pretty big patch, I can reduce it to smaller parts if you want,
I welcome comments questions
Yes, please split the API changes on a separate patch, and add the
corresponding changes at Documentation/DocBook/media/dvb to reflect
the needed API additions for Digicipher II and DSS.
Then, you can add one patch per logical change at the gp8psk driver.
git citool is your friend: using it, you can easily break the patch
into smaller ones.
Just keep in mind that the driver should keep compiling after each patch,
and try to maintain the one logical change per patch. Don't be shy
on the comments: we love comments on the patch descriptions, as they
can help a lot developers when they need to discover why the driver
stopped working, or what should be done on a similar driver to address
a common issue.
Btw, your emailer completely broke this patch: it broke long lines,
it replaced tabs by spaces, etc.
You should either use a different emailer, or to use git send-email
to send the patches directly to your smtp server.
Regards,
Mauro
Thanks
Signed-off-by: Chris Lee update...@gmail.com
--- media_tree/include/uapi/linux/dvb/frontend.h 2013-07-18
11:19:29.601746158 -0600
+++ media_tree.test/include/uapi/linux/dvb/frontend.h 2013-07-22
20:10:50.658719256 -0600
@@ -165,6 +165,7 @@
FEC_3_5,
FEC_9_10,
FEC_2_5,
+ FEC_5_11,
} fe_code_rate_t;
@@ -410,6 +411,10 @@
SYS_DVBT2,
SYS_TURBO,
SYS_DVBC_ANNEX_C,
+ SYS_DCII_C_QPSK,
+ SYS_DCII_I_QPSK,
+ SYS_DCII_Q_QPSK,
+ SYS_DCII_C_OQPSK,
} fe_delivery_system_t;
/* backward compatibility */
--- media_tree/drivers/media/usb/dvb-usb/gp8psk.h 2013-07-18
11:19:28.261746125 -0600
+++ media_tree.test/drivers/media/usb/dvb-usb/gp8psk.h 2013-07-22
19:54:19.642694697 -0600
@@ -52,6 +52,8 @@
#define GET_SERIAL_NUMBER 0x93/* in */
#define USE_EXTRA_VOLT 0x94
#define GET_FPGA_VERS 0x95
+#define GET_SIGNAL_STAT 0x9A
+#define GET_BER_RATE 0x9B
#define CW3K_INIT 0x9d
/* PSK_configuration bits */
--- media_tree/drivers/media/usb/dvb-usb/gp8psk-fe.c 2013-07-18
11:19:28.261746125 -0600
+++ media_tree.test/drivers/media/usb/dvb-usb/gp8psk-fe.c 2013-07-22
20:10:20.582718511 -0600
@@ -45,7 +45,8 @@
if (time_after(jiffies,st-next_status_check)) {
gp8psk_usb_in_op(st-d, GET_SIGNAL_LOCK, 0,0,st-lock,1);
gp8psk_usb_in_op(st-d, GET_SIGNAL_STRENGTH, 0,0,buf,6);
- st-snr = (buf[1]) 8 | buf[0];
+
+ st-snr = ((buf[1]) 8 | buf[0]) 4;
st-next_status_check = jiffies + (st-status_check_interval*HZ)/1000;
}
return 0;
@@ -53,7 +54,42 @@
static int gp8psk_fe_read_status(struct dvb_frontend* fe, fe_status_t
*status)
{
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
struct gp8psk_fe_state *st = fe-demodulator_priv;
+
+ u8 buf[32];
+ int frequency;
+ int carrier_error;
+ int carrier_offset;
+ int rate_error;
+ int rate_offset;
+ int symbol_rate;
+
+ int fe_gp8psk_system_return[] = {
+ SYS_DVBS,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_DCII_C_QPSK,
+ SYS_DCII_I_QPSK,
+ SYS_DCII_Q_QPSK,
+ SYS_DCII_C_OQPSK,
+ SYS_DSS,
+ SYS_UNDEFINED
+ };
+
+ int fe_gp8psk_modulation_return[] = {
+ QPSK,
+ QPSK,
+ PSK_8,
+ QAM_16,
+ QPSK,
+ QPSK,
+ QPSK,
+ QPSK,
+ QPSK,
+ };
+
gp8psk_fe_update_status(st);
if (st-lock)
@@ -61,18 +97,97 @@
else
*status = 0;
- if (*status FE_HAS_LOCK)
+ if (*status FE_HAS_LOCK) {
+ gp8psk_usb_in_op(st-d, GET_SIGNAL_STAT, 0, 0, buf, 32);
+ frequency = ((buf[11] 24) + (buf[10] 16) + (buf[9] 8) +
buf[8]) / 1000;
+ carrier_error = ((buf[15] 24) + (buf[14] 16) + (buf[13] 8)
+ buf[12]) / 1000;
+ carrier_offset = (buf[19] 24) + (buf[18] 16) + (buf[17] 8)
+ buf[16];
+ rate_error = (buf[23] 24) + (buf[22] 16) + (buf[21] 8) + buf[20];
+ rate_offset = (buf[27] 24) + (buf[26] 16) + (buf[25] 8) +
buf[24];
+ symbol_rate = (buf[31] 24) + (buf[30] 16) + (buf[29] 8) +
buf[28];
+
+ c-frequency = frequency - carrier_error;
+ c-symbol_rate = symbol_rate + rate_error;
+
+
[PATCH] This brings the genpix line of devices snr reporting in line with other drivers
Signed-off-by: Chris Lee update...@gmail.com
---
drivers/media/usb/dvb-usb/gp8psk-fe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb/gp8psk-fe.c
b/drivers/media/usb/dvb-usb/gp8psk-fe.c
index 67957dd..5864f37 100644
--- a/drivers/media/usb/dvb-usb/gp8psk-fe.c
+++ b/drivers/media/usb/dvb-usb/gp8psk-fe.c
@@ -45,7 +45,7 @@ static int gp8psk_fe_update_status(struct gp8psk_fe_state *st)
if (time_after(jiffies,st-next_status_check)) {
gp8psk_usb_in_op(st-d, GET_SIGNAL_LOCK, 0,0,st-lock,1);
gp8psk_usb_in_op(st-d, GET_SIGNAL_STRENGTH, 0,0,buf,6);
- st-snr = (buf[1]) 8 | buf[0];
+ st-snr = ((buf[1]) 8 | buf[0]) 4;
st-next_status_check = jiffies +
(st-status_check_interval*HZ)/1000;
}
return 0;
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
Yes. I think we could use i2c_board_info for passing platform data.
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
It is how the PHY framework finds a PHY, when the controller (say USB)requests
a PHY from the PHY framework.
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
I think instead we can use the same data while creating the platform data of
the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also pass the
*data* it actually got from the platform data to the framework.
The PHY user driver (USB), while requesting for the PHY (from the PHY
framework) will pass the *data* it got from its platform data.
The PHY framework can do a comparison of the *data* pointers it has and return
the appropriate PHY to the controller.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
I meant something like this
struct phy_info {
const char *name;
};
struct phy_platform_data {
.
.
struct phy_info *info;
};
struct usb_controller_platform_data {
.
.
struct phy_info *info;
};
struct phy_info phy_info;
While creating the phy device
struct phy_platform_data phy_data;
phy_data.info = info;
platform_device_add_data(pdev, phy_data, sizeof(*phy_data))
platform_device_add();
While creating the controller device
struct usb_controller_platform_data controller_data;
controller_data.info = info;
platform_device_add_data(pdev, controller_data,
sizeof(*controller_data))
platform_device_add();
Then modify PHY framework API phy create
phy_create((struct device *dev, const struct phy_ops
[PATCH] gp8psk: Implement gp8psk_fe_read_ber
Signed-off-by: Chris Lee update...@gmail.com
---
drivers/media/usb/dvb-usb/gp8psk-fe.c | 13 ++---
drivers/media/usb/dvb-usb/gp8psk.h| 1 +
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/gp8psk-fe.c
b/drivers/media/usb/dvb-usb/gp8psk-fe.c
index 5864f37..223a3ca 100644
--- a/drivers/media/usb/dvb-usb/gp8psk-fe.c
+++ b/drivers/media/usb/dvb-usb/gp8psk-fe.c
@@ -68,11 +68,18 @@ static int gp8psk_fe_read_status(struct dvb_frontend* fe,
fe_status_t *status)
return 0;
}
-/* not supported by this Frontend */
static int gp8psk_fe_read_ber(struct dvb_frontend* fe, u32 *ber)
{
- (void) fe;
- *ber = 0;
+ struct gp8psk_fe_state *st = fe-demodulator_priv;
+
+ u8 buf[4];
+
+ if (gp8psk_usb_in_op(st-d, GET_BER_RATE, 0, 0, buf, 4)) {
+ return -EINVAL;
+ }
+
+ *ber = (buf[3] 24) + (buf[2] 16) + (buf[1] 8) + buf[0];
+
return 0;
}
diff --git a/drivers/media/usb/dvb-usb/gp8psk.h
b/drivers/media/usb/dvb-usb/gp8psk.h
index ed32b9d..ff6bb3c 100644
--- a/drivers/media/usb/dvb-usb/gp8psk.h
+++ b/drivers/media/usb/dvb-usb/gp8psk.h
@@ -52,6 +52,7 @@ extern int dvb_usb_gp8psk_debug;
#define GET_SERIAL_NUMBER 0x93/* in */
#define USE_EXTRA_VOLT 0x94
#define GET_FPGA_VERS 0x95
+#define GET_BER_RATE 0x9B
#define CW3K_INIT 0x9d
/* PSK_configuration bits */
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/5] v4l2-async DT support improvement and cleanups
Hi Sylwester, On Mon, Jul 22, 2013 at 11:34 PM, Sylwester Nawrocki s.nawro...@samsung.com wrote: Hello, This is a few patches for the v4l2-async API I wrote while adding the asynchronous subdev registration support to the exynos4-is driver. The most significant change is addition of V4L2_ASYNC_MATCH_OF subdev matching method, where host driver can pass a list of of_node pointers identifying its subdevs. I thought it's a reasonable and simple enough way to support device tree based systems. Comments/other ideas are of course welcome. Thanks, Sylwester Sylwester Nawrocki (5): V4L2: Drop bus_type check in v4l2-async match functions V4L2: Rename v4l2_async_bus_* to v4l2_async_match_* V4L2: Add V4L2_ASYNC_MATCH_OF subdev matching type V4L2: Rename subdev field of struct v4l2_async_notifier V4L2: Fold struct v4l2_async_subdev_list with struct v4l2_subdev Thanks for the patche's tested on DA850 EVM for VPIF driver. for patches 1,2,4,5: Acked-and-tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com and for patch 3: Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 4/5] V4L2: Rename subdev field of struct v4l2_async_notifier
Hi Sylwester,
On Mon, Jul 22, 2013 at 11:34 PM, Sylwester Nawrocki
s.nawro...@samsung.com wrote:
This is a purely cosmetic change. Since the 'subdev' member
points to an array of subdevs it seems more intuitive to name
it in plural form.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/soc_camera/soc_camera.c |2 +-
drivers/media/v4l2-core/v4l2-async.c |2 +-
include/media/v4l2-async.h |4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
can you include the following changes in the same patch ?
so that git bisect doesn’t break.
(maybe you need to rebase the patches on
http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/for-v3.12)
Regards,
--Prabhakar Lad
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index b11d7a7..7fbde6d 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -2168,7 +2168,7 @@ static __init int vpif_probe(struct platform_device *pdev)
}
vpif_probe_complete();
} else {
- vpif_obj.notifier.subdev = vpif_obj.config-asd;
+ vpif_obj.notifier.subdevs = vpif_obj.config-asd;
vpif_obj.notifier.num_subdevs = vpif_obj.config-asd_sizes[0];
vpif_obj.notifier.bound = vpif_async_bound;
vpif_obj.notifier.complete = vpif_async_complete;
diff --git a/drivers/media/platform/davinci/vpif_display.c
b/drivers/media/platform/davinci/vpif_display.c
index c2ff067..6336dfc 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1832,7 +1832,7 @@ static __init int vpif_probe(struct platform_device *pdev)
}
vpif_probe_complete();
} else {
- vpif_obj.notifier.subdev = vpif_obj.config-asd;
+ vpif_obj.notifier.subdevs = vpif_obj.config-asd;
vpif_obj.notifier.num_subdevs = vpif_obj.config-asd_sizes[0];
vpif_obj.notifier.bound = vpif_async_bound;
vpif_obj.notifier.complete = vpif_async_complete;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gp8psk: add systems supported by genpix devices to .delsys
---
drivers/media/usb/dvb-usb/gp8psk-fe.c | 2 +-
include/uapi/linux/dvb/frontend.h | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb/gp8psk-fe.c
b/drivers/media/usb/dvb-usb/gp8psk-fe.c
index 223a3ca..fcdf82c 100644
--- a/drivers/media/usb/dvb-usb/gp8psk-fe.c
+++ b/drivers/media/usb/dvb-usb/gp8psk-fe.c
@@ -333,7 +333,7 @@ success:
static struct dvb_frontend_ops gp8psk_fe_ops = {
- .delsys = { SYS_DVBS },
+ .delsys = { SYS_DCII_C_QPSK, SYS_DCII_I_QPSK, SYS_DCII_Q_QPSK,
SYS_DCII_C_OQPSK, SYS_DSS, SYS_DVBS2, SYS_TURBO, SYS_DVBS },
.info = {
.name = Genpix DVB-S,
.frequency_min = 80,
diff --git a/include/uapi/linux/dvb/frontend.h
b/include/uapi/linux/dvb/frontend.h
index c56d77c..ada08a8 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -410,6 +410,10 @@ typedef enum fe_delivery_system {
SYS_DVBT2,
SYS_TURBO,
SYS_DVBC_ANNEX_C,
+ SYS_DCII_C_QPSK,
+ SYS_DCII_I_QPSK,
+ SYS_DCII_Q_QPSK,
+ SYS_DCII_C_OQPSK,
} fe_delivery_system_t;
/* backward compatibility */
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gp8psk: add systems supported by genpix devices to .delsys
Genpix Skywalker and 8psk-to-usb devices do not support dvb-s2. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] gp8psk: update gp8psk_fe_read_status to return actual tuned values, correct fec/sr/freq etc
---
drivers/media/usb/dvb-usb/gp8psk-fe.c | 111 +-
drivers/media/usb/dvb-usb/gp8psk.h| 1 +
include/uapi/linux/dvb/frontend.h | 1 +
3 files changed, 111 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/gp8psk-fe.c
b/drivers/media/usb/dvb-usb/gp8psk-fe.c
index fcdf82c..4e61c48 100644
--- a/drivers/media/usb/dvb-usb/gp8psk-fe.c
+++ b/drivers/media/usb/dvb-usb/gp8psk-fe.c
@@ -53,7 +53,42 @@ static int gp8psk_fe_update_status(struct gp8psk_fe_state
*st)
static int gp8psk_fe_read_status(struct dvb_frontend* fe, fe_status_t *status)
{
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
struct gp8psk_fe_state *st = fe-demodulator_priv;
+
+ u8 buf[32];
+ int frequency;
+ int carrier_error;
+ int carrier_offset;
+ int rate_error;
+ int rate_offset;
+ int symbol_rate;
+
+ int fe_gp8psk_system_return[] = {
+ SYS_DVBS,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_DCII_C_QPSK,
+ SYS_DCII_I_QPSK,
+ SYS_DCII_Q_QPSK,
+ SYS_DCII_C_OQPSK,
+ SYS_DSS,
+ SYS_UNDEFINED
+ };
+
+ int fe_gp8psk_modulation_return[] = {
+ QPSK,
+ QPSK,
+ PSK_8,
+ QAM_16,
+ QPSK,
+ QPSK,
+ QPSK,
+ QPSK,
+ QPSK,
+ };
+
gp8psk_fe_update_status(st);
if (st-lock)
@@ -61,10 +96,82 @@ static int gp8psk_fe_read_status(struct dvb_frontend* fe,
fe_status_t *status)
else
*status = 0;
- if (*status FE_HAS_LOCK)
+ if (*status FE_HAS_LOCK) {
+ gp8psk_usb_in_op(st-d, GET_SIGNAL_STAT, 0, 0, buf, 32);
+ frequency = ((buf[11] 24) + (buf[10] 16) +
(buf[9] 8) + buf[8]) / 1000;
+ carrier_error = ((buf[15] 24) + (buf[14] 16) + (buf[13]
8) + buf[12]) / 1000;
+ carrier_offset = (buf[19] 24) + (buf[18] 16) + (buf[17]
8) + buf[16];
+ rate_error = (buf[23] 24) + (buf[22] 16) +
(buf[21] 8) + buf[20];
+ rate_offset = (buf[27] 24) + (buf[26] 16) +
(buf[25] 8) + buf[24];
+ symbol_rate = (buf[31] 24) + (buf[30] 16) +
(buf[29] 8) + buf[28];
+
+ c-frequency= frequency - carrier_error;
+ c-symbol_rate = symbol_rate + rate_error;
+
+ switch (c-delivery_system) {
+ case SYS_DSS:
+ case SYS_DVBS:
+ c-delivery_system =
fe_gp8psk_system_return[buf[1]];
+ c-modulation =
fe_gp8psk_modulation_return[buf[1]];
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_1_2; break;
+ case 1: c-fec_inner = FEC_2_3; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_5_6; break;
+ case 4: c-fec_inner = FEC_6_7; break;
+ case 5: c-fec_inner = FEC_7_8; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ break;
+ case SYS_TURBO:
+ c-delivery_system =
fe_gp8psk_system_return[buf[1]];
+ c-modulation =
fe_gp8psk_modulation_return[buf[1]];
+ if (c-modulation == QPSK) {
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_7_8; break;
+ case 1: c-fec_inner = FEC_1_2; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_2_3; break;
+ case 4: c-fec_inner = FEC_5_6; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ } else {
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_2_3; break;
+ case 1: c-fec_inner = FEC_3_4; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_5_6; break;
+ case 4: c-fec_inner = FEC_8_9; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ }
+ break;
+ case SYS_DCII_C_QPSK:
+ case SYS_DCII_I_QPSK:
+ case SYS_DCII_Q_QPSK:
+ case SYS_DCII_C_OQPSK:
+ c-modulation
Re: [PATCH] gp8psk: add systems supported by genpix devices to .delsys
Correct, but many older userland applications used SYS_DVBS2 to tune
before SYS_TURBO was added. I have no problem removing it but others
might.
from gp8psk-fe.c
switch (c-delivery_system) {
case SYS_DVBS:
if (c-modulation != QPSK) {
deb_fe(%s: unsupported modulation selected (%d)\n,
__func__, c-modulation);
return -EOPNOTSUPP;
}
c-fec_inner = FEC_AUTO;
break;
case SYS_DVBS2: /* kept for backwards compatibility */
deb_fe(%s: DVB-S2 delivery system selected\n, __func__);
break;
case SYS_TURBO:
deb_fe(%s: Turbo-FEC delivery system selected\n, __func__);
break;
Chris
On Tue, Jul 23, 2013 at 9:53 AM, VDR User user@gmail.com wrote:
Genpix Skywalker and 8psk-to-usb devices do not support dvb-s2.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: width and height of JPEG compressed images
Hi Sylwester, On 22 July 2013 22:48 Sylwester Nawrocki wrote: On 07/15/2013 11:18 AM, Thomas Vajzovic wrote: On 10 July 2013 20:44 Sylwester Nawrocki wrote: On 07/07/2013 10:18 AM, Thomas Vajzovic wrote: On 06 July 2013 20:58 Sylwester Nawrocki wrote: On 07/05/2013 10:22 AM, Thomas Vajzovic wrote: The hardware reads AxB sensor pixels from its array, resamples them to CxD image pixels, and then compresses them to ExF bytes. Yes you are correct that the sensor zero pads the compressed data to a fixed size. That size must be specified in two separate registers, called spoof width and spoof height. Above CxD is the image size after binning/skipping and resizing, ExF is the spoof size. All right. We need to make it clear that the format on video node refers to data in memory, while media bus format/frame descriptor specifies how data is transmitted on the physical bus. When there is scaling, etc. involved on the bridge side relations between the two are not that straightforward. sizeimage / bytesperline needs to be translatable to frame descriptor/media bus format information and the other way around. I'm not sure that translating them is reasonable. The image width and height are one thing, and the data size (whether 1D or 2D) is another thing. They just need to be expressed explicitly. Secondly, the pair of numbers (E,F) in my case have exaclty the same meaning and are used in exactly the same way as the single number (sizeimage) which is used in the cameras that use the current API. Logically the two numbers should be passed around and set and modified in all the same places that sizeimage currently is, but as a tuple. The two cannot be separated with one set using one API and the other a different API. Sure, we just need to think of how to express (E, F) in the frame descriptors API and teach the bridge driver to use it. As Sakari mentioned width, height and bpp is probably going to be sufficient. Bits-per-image-pixel is variable, but I assume you mean average- bits-per-image-pixel. This is confusing and inexact: What if the user wants to compress 800x600 to 142kB? then bpp = 2.4234667. This number doesn't really mean very much, and how would you express it so that the bridge always get exact pair of integers that the sensor chose without rounding error? I suggest that the clean and sensible solution is to explicitly express physical width, with physical-bits- per-pixel = always 8 (assuming FMT_JPEG_1X8). Many thanks for your time on this. Please see also my reply at Mon 22/07/2013 09:41. Your proposal above sounds sane, I've seen already 1D/2D DMA notations in some documentation. Is datasheet of your bridge device available publicly ? Which Blackfin processor is that ? http://www.analog.com/static/imported-files/processor_manuals/ADSP-BF51x_hwr_rev1.2.pdf Best regards, Tom -- Mr T. Vajzovic Software Engineer Infrared Integrated Systems Ltd Visit us at www.irisys.co.uk Disclaimer: This e-mail message is confidential and for use by the addressee only. If the message is received by anyone other than the addressee, please return the message to the sender by replying to it and then delete the original message and the sent message from your computer. Infrared Integrated Systems Limited Park Circle Tithe Barn Way Swan Valley Northampton NN4 9BG Registration Number: 3186364. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gp8psk: add DCII and DSS tuning to set_frontend, fix fec tuning values for everything else
fix: DVB-S QPSK and TURBO QPSK have different fec tuning values
add: DSS
add: DCII
Signed-off-by: Chris Lee update...@gmail.com
---
drivers/media/usb/dvb-usb/gp8psk-fe.c | 180 ++
1 file changed, 116 insertions(+), 64 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/gp8psk-fe.c
b/drivers/media/usb/dvb-usb/gp8psk-fe.c
index 4e61c48..4175cf0 100644
--- a/drivers/media/usb/dvb-usb/gp8psk-fe.c
+++ b/drivers/media/usb/dvb-usb/gp8psk-fe.c
@@ -235,93 +235,145 @@ static int gp8psk_fe_set_frontend(struct dvb_frontend
*fe)
u32 freq = c-frequency * 1000;
int gp_product_id = le16_to_cpu(state-d-udev-descriptor.idProduct);
- deb_fe(%s()\n, __func__);
+ info(%s() freq: %d, sr: %d, __func__, freq, c-symbol_rate);
+ cmd[0] = c-symbol_rate 0xff;
+ cmd[1] = (c-symbol_rate 8) 0xff;
+ cmd[2] = (c-symbol_rate 16) 0xff;
+ cmd[3] = (c-symbol_rate 24) 0xff;
cmd[4] = freq 0xff;
cmd[5] = (freq 8) 0xff;
cmd[6] = (freq 16) 0xff;
cmd[7] = (freq 24) 0xff;
- /* backwards compatibility: DVB-S + 8-PSK were used for Turbo-FEC */
- if (c-delivery_system == SYS_DVBS c-modulation == PSK_8)
+ /* backwards compatibility: DVB-S2 used to be used for Turbo-FEC */
+ if (c-delivery_system == SYS_DVBS2)
c-delivery_system = SYS_TURBO;
switch (c-delivery_system) {
case SYS_DVBS:
- if (c-modulation != QPSK) {
- deb_fe(%s: unsupported modulation selected (%d)\n,
+ info(%s: DVB-S delivery system selected w/fec %d, __func__,
c-fec_inner);
+ c-fec_inner = FEC_AUTO;
+ cmd[8] = ADV_MOD_DVB_QPSK;
+ cmd[9] = 5;
+ break;
+ case SYS_TURBO:
+ info(%s: Turbo-FEC delivery system selected, __func__);
+ switch (c-modulation) {
+ case QPSK:
+ info(%s: modulation QPSK selected w/fec %d, __func__,
c-fec_inner);
+ cmd[8] = ADV_MOD_TURBO_QPSK;
+ switch (c-fec_inner) {
+ case FEC_1_2: cmd[9] = 1; break;
+ case FEC_2_3: cmd[9] = 3; break;
+ case FEC_3_4: cmd[9] = 2; break;
+ case FEC_5_6: cmd[9] = 4; break;
+ default:cmd[9] = 0; break;
+ }
+ break;
+ case PSK_8:
+ info(%s: modulation 8PSK selected w/fec %d, __func__,
c-fec_inner);
+ cmd[8] = ADV_MOD_TURBO_8PSK;
+ switch (c-fec_inner) {
+ case FEC_2_3: cmd[9] = 0; break;
+ case FEC_3_4: cmd[9] = 1; break;
+ case FEC_3_5: cmd[9] = 2; break;
+ case FEC_5_6: cmd[9] = 3; break;
+ case FEC_8_9: cmd[9] = 4; break;
+ default:cmd[9] = 0; break;
+ }
+ break;
+ case QAM_16: /* QAM_16 is for compatibility with DN */
+ info(%s: modulation QAM_16 selected w/fec %d,
__func__, c-fec_inner);
+ cmd[8] = ADV_MOD_TURBO_16QAM;
+ cmd[9] = 0;
+ break;
+ default: /* Unknown modulation */
+ info(%s: unsupported modulation selected (%d),
__func__, c-modulation);
return -EOPNOTSUPP;
}
- c-fec_inner = FEC_AUTO;
break;
- case SYS_DVBS2: /* kept for backwards compatibility */
- deb_fe(%s: DVB-S2 delivery system selected\n, __func__);
+ case SYS_DSS:
+ info(%s: DSS delivery system selected w/fec %d, __func__,
c-fec_inner);
+ cmd[8] = ADV_MOD_DSS_QPSK;
+ switch (c-fec_inner) {
+ case FEC_1_2: cmd[9] = 0; break;
+ case FEC_2_3: cmd[9] = 1; break;
+ case FEC_3_4: cmd[9] = 2; break;
+ case FEC_5_6: cmd[9] = 3; break;
+ case FEC_7_8: cmd[9] = 4; break;
+ case FEC_AUTO: cmd[9] = 5; break;
+ case FEC_6_7: cmd[9] = 6; break;
+ default:cmd[9] = 5; break;
+ }
break;
- case SYS_TURBO:
- deb_fe(%s: Turbo-FEC delivery system selected\n, __func__);
+ case SYS_DCII_C_QPSK:
+ info(%s: DCII_C_QPSK delivery system selected w/fec %d,
__func__, c-fec_inner);
+ cmd[8] = ADV_MOD_DCII_C_QPSK;
+ switch (c-fec_inner) {
+ /* 5/11 FEC is cmd[9] = 0 but not added to the API */
+ case FEC_1_2: cmd[9] = 1; break;
+
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
Yes. I think we could use i2c_board_info for passing platform data.
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
It is how the PHY framework finds a PHY, when the controller (say USB)requests
a PHY from the PHY framework.
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
I think instead we can use the same data while creating the platform data of
the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also pass the
*data* it actually got from the platform data to the framework.
The PHY user driver (USB), while requesting for the PHY (from the PHY
framework) will pass the *data* it got from its platform data.
The PHY framework can do a comparison of the *data* pointers it has and return
the appropriate PHY to the controller.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
I meant something like this
struct phy_info {
const char *name;
};
struct phy_platform_data {
.
.
struct phy_info *info;
};
struct usb_controller_platform_data {
.
.
struct phy_info *info;
};
struct phy_info phy_info;
While creating the phy device
struct phy_platform_data phy_data;
phy_data.info = info;
platform_device_add_data(pdev, phy_data, sizeof(*phy_data))
platform_device_add();
While creating the controller device
struct usb_controller_platform_data controller_data;
controller_data.info = info;
Re: [PATCH] gp8psk: add systems supported by genpix devices to .delsys
Correct, but many older userland applications used SYS_DVBS2 to tune before SYS_TURBO was added. I have no problem removing it but others might. I think the best solution here would be not to put false info in the driver and notify the author(s) of any apps still not updated to use SYS_TURBO, that they need to do so or let the communities for those apps fix it. Having the Genpix say it's dvb-s2 capable when it isn't can be a problem in systems with actual dvb-s2 sources which is why, iirc, SYS_TURBO was added in the first place. If nobody wants to bother fixing those apps (to my knowledge its only mythtv that has this problem) correctly and still wants to rely on the driver misrepresenting the devices capabilities then it seems appropriate that should be done in an external patch since SYS_TURBO already exists to prevent this. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi Greg,
On Tuesday 23 July 2013 09:48 PM, Greg KH wrote:
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
.
.
snip
.
.
static struct phy *phy_lookup(void *priv) {
.
.
if (phy-priv==priv) //instead of string comparison, we'll use
pointer
return phy;
}
PHY driver should be like
phy_create((dev, ops, pdata-info);
The controller driver would do
phy_get(dev, NULL, pdata-info);
Now the PHY framework will check for a match of *priv* pointer and return
the PHY.
I think this should be possible?
Ick, no. Why can't you just pass the pointer to the phy itself? If you
had a priv pointer to search from, then you could have just passed the
original phy pointer in the first place, right?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
Please just pass the real phy pointer around, that's what it is there
for. Your board binding logic/code should be able to handle this, as
it somehow was going to do the same thing with a name.
The problem is the board file won't have the *phy* pointer. *phy* pointer is
created at a much later time when the phy driver is probed.
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 09:58:34PM +0530, Kishon Vijay Abraham I wrote:
Hi Greg,
On Tuesday 23 July 2013 09:48 PM, Greg KH wrote:
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
.
.
snip
.
.
static struct phy *phy_lookup(void *priv) {
.
.
if (phy-priv==priv) //instead of string comparison, we'll use
pointer
return phy;
}
PHY driver should be like
phy_create((dev, ops, pdata-info);
The controller driver would do
phy_get(dev, NULL, pdata-info);
Now the PHY framework will check for a match of *priv* pointer and return
the PHY.
I think this should be possible?
Ick, no. Why can't you just pass the pointer to the phy itself? If you
had a priv pointer to search from, then you could have just passed the
original phy pointer in the first place, right?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
Please just pass the real phy pointer around, that's what it is there
for. Your board binding logic/code should be able to handle this, as
it somehow was going to do the same thing with a name.
The problem is the board file won't have the *phy* pointer. *phy* pointer is
created at a much later time when the phy driver is probed.
Ok, then save it then, as no one could have used it before then, right?
All I don't want to see is any get by name/void * functions in the
api, as that way is fragile and will break, as people have already
shown.
Just pass the real pointer around. If that is somehow a problem, then
something larger is a problem with how board devices are tied together :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
Not all tuners support all fec's - genpix devices support an odd 5/11 fec for digicipher, pretty sure no one else does. - stv0899 supports 1/2, 2/3, 3/4, 5/6, 6/7, 7/8 - stv0900 supports 1/2, 3/5, 2/3, 3/4, 4/5, 5/6, 8/9, 9/10 Not all tuners support the entire range of fec's. I think this is more the norm then the exception. If the userland application can poll the driver for a list of supported fec it allows them to have a list of valid tuning options for the user to choose from, vs listing everything and hoping it doesnt fail. As stated Id much rather have a list made up from system - modulation - fec. ie genpix SYS_TURBO - QPSK/8PSK SYS_TURBO.QPSK - 1/2, 2/3, 3/4, 5/6, 7/8 SYS_TURBO.8PSK - 2/3, 3/4, 5/6, 8/9 but that could get more complicated to implement pretty quickly Chris Lee On Tue, Jul 23, 2013 at 7:35 AM, Manu Abraham abraham.m...@gmail.com wrote: On Sat, Jul 20, 2013 at 1:57 AM, Chris Lee update...@gmail.com wrote: In frontend.h we have a struct called dvb_frontend_ops, in there we have an element called delsys to show the delivery systems supported by the tuner, Id like to propose we add onto that with delmod and delfec. Its not a perfect solution as sometimes a specific modulation or fec is only availible on specific systems. But its better then what we have now. The struct fe_caps isnt really suited for this, its missing many systems, modulations, and fec's. Its just not expandable enough to get all the supported sys/mod/fec a tuner supports in there. Question Why should an application know all the modulations and FEC's supported by a demodulator ? Aren't demodulators compliant to their respective delivery systems ? Or do you mean to state that, you are trying to work around some demodulator quirks ? Regards, Manu -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 09:18:46 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever
parent?
Yes. I think we could use i2c_board_info for passing platform data.
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even
posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
It is how the PHY framework finds a PHY, when the controller (say
USB)requests a PHY from the PHY framework.
In this case, it doesn't matter where the platform_device
structures
are created or where the driver source code is. Let's take a
simple
example. Suppose the system design includes a PHY named foo.
Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that;
make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the
same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation -
you
export random driver-specific symbols from one driver to another.
Then
No, that's not what I said. Neither the PHY driver nor the
controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
I think instead we can use the same data while creating the platform
data of the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also
pass the *data* it actually got from the platform data to the
framework. The PHY user driver (USB), while requesting for the PHY
(from the PHY framework) will pass the *data* it got from its platform
data.
The PHY framework can do a comparison of the *data* pointers it has and
return the appropriate PHY to the controller.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2
and
there are two types of consumer drivers (e.g. USB host
controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same
name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file
must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
I meant something like this
struct phy_info {
const char *name;
};
struct phy_platform_data {
.
.
struct phy_info *info;
};
struct usb_controller_platform_data {
.
.
struct phy_info *info;
};
struct phy_info phy_info;
While creating the phy device
struct phy_platform_data phy_data;
Re: [PATCH] gp8psk: add systems supported by genpix devices to .delsys
Ive got no problem pulling it out, doesnt affect anything on my end. Do I need to resubmit the patch ? Chris Lee On Tue, Jul 23, 2013 at 10:22 AM, VDR User user@gmail.com wrote: Correct, but many older userland applications used SYS_DVBS2 to tune before SYS_TURBO was added. I have no problem removing it but others might. I think the best solution here would be not to put false info in the driver and notify the author(s) of any apps still not updated to use SYS_TURBO, that they need to do so or let the communities for those apps fix it. Having the Genpix say it's dvb-s2 capable when it isn't can be a problem in systems with actual dvb-s2 sources which is why, iirc, SYS_TURBO was added in the first place. If nobody wants to bother fixing those apps (to my knowledge its only mythtv that has this problem) correctly and still wants to rely on the driver misrepresenting the devices capabilities then it seems appropriate that should be done in an external patch since SYS_TURBO already exists to prevent this. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Prof DVB-S2 USB device
#Sorry for sending to individual email ids Hi, I am trying to use Prof DVB-S2 USB device with Linux host. Device gets detected. But, I am facing the following problems. 1. It takes approximately 21 minutes to get /dev/dvb/adapter0/frontend0 and /dev/dvb/adapter0/demux0 to get created. This happens every time 2. After /dev/dvb/adapter0/frontend0 gets created, when I use w_scan utility to scan for channels, it does not list the channels. a. In dmesg logs, I see DEMOD LOCK FAIL error continuously. Can you please help me? Regards, Kishore. SASKEN BUSINESS DISCLAIMER: This message may contain confidential, proprietary or legally privileged information. In case you are not the original intended Recipient of the message, you must not, directly or indirectly, use, disclose, distribute, print, or copy any part of this message and you are requested to delete it and inform the sender. Any views expressed in this message are those of the individual sender unless otherwise stated. Nothing contained in this message shall be construed as an offer or acceptance of any offer by Sasken Communication Technologies Limited (Sasken) unless sent with that express intent and with due authority of Sasken. Sasken has taken enough precautions to prevent the spread of viruses. However the company accepts no liability for any damage caused by any virus transmitted by this email. Read Disclaimer at http://www.sasken.com/extras/mail_disclaimer.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BRAINSTORM] Controls, matrices and properties
Hi Laurent, On Thu, Jul 18, 2013 at 03:07:26AM +0200, Laurent Pinchart wrote: ... An unrelated thing, which is out of scope for now, but something to think about: when passing around large amounts of (configuration) data the number of times the data is copied really counts. Especially on embedded systems. Memory mapping helps avoiding problems --- what would happen is that the driver would access memory mapped to the device directly and the driver would then get the address to pass to the device as the configuration. Like video buffers, but for control, not data. Would you map that memory to userspace as well ? Yes --- that's the very intent. This way all unnecessary memory copies in the kernel can be avoided. Say, if you're providing (and copying) the device a new LSC table of 128 kiB for every frame while recording video, it will show up in the energy consumption of the system. It does let the user space to do wrong things, too. But the same is equally true for video buffers as well. -- Cheers, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Jul 23 19:00:18 CEST 2013 git branch: test git hash: c859e6ef33ac0c9a5e9e934fe11a2232752b4e96 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: v0.4.5-rc1 host hardware: x86_64 host os:3.9-7.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: ERRORS linux-2.6.32.27-i686: ERRORS linux-2.6.33.7-i686: ERRORS linux-2.6.34.7-i686: ERRORS linux-2.6.35.9-i686: ERRORS linux-2.6.36.4-i686: ERRORS linux-2.6.37.6-i686: ERRORS linux-2.6.38.8-i686: ERRORS linux-2.6.39.4-i686: ERRORS linux-3.0.60-i686: ERRORS linux-3.10-i686: ERRORS linux-3.1.10-i686: ERRORS linux-3.2.37-i686: ERRORS linux-3.3.8-i686: ERRORS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: ERRORS linux-3.6.11-i686: ERRORS linux-3.7.4-i686: ERRORS linux-3.8-i686: ERRORS linux-3.9.2-i686: ERRORS linux-2.6.31.14-x86_64: ERRORS linux-2.6.32.27-x86_64: ERRORS linux-2.6.33.7-x86_64: ERRORS linux-2.6.34.7-x86_64: ERRORS linux-2.6.35.9-x86_64: ERRORS linux-2.6.36.4-x86_64: ERRORS linux-2.6.37.6-x86_64: ERRORS linux-2.6.38.8-x86_64: ERRORS linux-2.6.39.4-x86_64: ERRORS linux-3.0.60-x86_64: ERRORS linux-3.10-x86_64: ERRORS linux-3.1.10-x86_64: ERRORS linux-3.2.37-x86_64: ERRORS linux-3.3.8-x86_64: ERRORS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: ERRORS linux-3.6.11-x86_64: ERRORS linux-3.7.4-x86_64: ERRORS linux-3.8-x86_64: ERRORS linux-3.9.2-x86_64: ERRORS apps: WARNINGS spec-git: OK sparse version: v0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself? If you
had a priv pointer to search from, then you could have just passed the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but let's try to
check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data structure
unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call to
phy_create(). Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in some, not
so uncommon cases, but there is already a lot of existing lookup solutions
over the kernel and so there is no point in introducing another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.
Please just pass the real phy pointer around, that's what it is there
for. Your board binding logic/code should be able to handle this, as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't really nice,
because it's a layering violation (at least if I understood what you mean).
This is because you need to have full definition of struct phy in board file
and a structure that is used as private data in PHY core comes from
platform code.
No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.
Does that make more sense?
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 10:37:05AM -0400, Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: Okay. Are PHYs _always_ platform devices? They can be i2c, spi or any other device types as well. In those other cases, presumably there is no platform data associated with the PHY since it isn't a platform device. Then how does the kernel know which controller is attached to the PHY? Is this spelled out in platform data associated with the PHY's i2c/spi/whatever parent? Platform data is nothing to do with the platform bus - it's board specific data (ie, data for the platform) and can be done with any device. signature.asc Description: Digital signature
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 10:37:11AM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: I fully agree that a simple, single string will not scale even in some, not so uncommon cases, but there is already a lot of existing lookup solutions over the kernel and so there is no point in introducing another one. I'm trying to get _rid_ of lookup solutions and just use a real pointer, as you should. I'll go tackle those other ones after this one is taken care of, to show how the others should be handled as well. What are the problems you are seeing with doing things with lookups? Having to write platform data for everything gets old fast and the code duplication is pretty tedious... signature.asc Description: Digital signature
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself? If
you
had a priv pointer to search from, then you could have just passed
the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but let's
try to check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are
really helpful.
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
---
-8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data structure
unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call to
phy_create(). Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).
Hmm? So, do you suggest to call phy_create() from board file? What phy_ops
struct and other hardware parameters would it take?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in some,
not so uncommon cases, but there is already a lot of existing lookup
solutions over the kernel and so there is no point in introducing
another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.
There was a reason for introducing lookup solutions. The reason was that in
board file there is no way to get a pointer to something that is going to be
created much later in time. We don't do time travel ;-).
Please just pass the real phy pointer around, that's what it is
there
for. Your board binding logic/code should be able to handle this,
as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't really
nice, because it's a layering violation (at least if I understood what
you mean). This is because you need to have full definition of struct
phy in board file and a structure that is used as private data in PHY
core comes from platform code.
No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.
Does that make more sense?
Well, to the point that I think I now understood your suggestion.
Unfortunately the suggestion alone isn't really something that can be done,
considering how driver core and generic frameworks work.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 06:44:56PM +0100, Mark Brown wrote: On Tue, Jul 23, 2013 at 10:37:11AM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: I fully agree that a simple, single string will not scale even in some, not so uncommon cases, but there is already a lot of existing lookup solutions over the kernel and so there is no point in introducing another one. I'm trying to get _rid_ of lookup solutions and just use a real pointer, as you should. I'll go tackle those other ones after this one is taken care of, to show how the others should be handled as well. What are the problems you are seeing with doing things with lookups? You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/27] drivers/media/platform: don't check resource with devm_ioremap_resource
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.
Signed-off-by: Wolfram Sang w...@the-dreams.de
---
Please apply via the subsystem-tree.
drivers/media/platform/coda.c |5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index df4ada88..6ea4717 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -2032,11 +2032,6 @@ static int coda_probe(struct platform_device *pdev)
/* Get memory for physical registers */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res == NULL) {
- dev_err(pdev-dev, failed to get memory region resource\n);
- return -ENOENT;
- }
-
dev-regs_base = devm_ioremap_resource(pdev-dev, res);
if (IS_ERR(dev-regs_base))
return PTR_ERR(dev-regs_base);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/3] [media] coda: Check the return value from clk_prepare_enable()
clk_prepare_enable() may fail, so let's check its return value and propagate it in the case of error. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v3: - Adapt it to make error handling easier Changes since v2: - Release the previously acquired resources Changes since v1: - Add missing 'if' drivers/media/platform/coda.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 9384f02..2d1576b 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -1531,8 +1531,13 @@ static int coda_open(struct file *file) ctx-dev = dev; ctx-idx = idx; - clk_prepare_enable(dev-clk_per); - clk_prepare_enable(dev-clk_ahb); + ret = clk_prepare_enable(dev-clk_per); + if (ret) + goto err_clk_per; + + ret = clk_prepare_enable(dev-clk_ahb); + if (ret) + goto err_clk_ahb; set_default_params(ctx); ctx-m2m_ctx = v4l2_m2m_ctx_init(dev-m2m_dev, ctx, @@ -1575,7 +1580,9 @@ err_ctrls_setup: v4l2_m2m_ctx_release(ctx-m2m_ctx); err_ctx_init: clk_disable_unprepare(dev-clk_ahb); +err_clk_ahb: clk_disable_unprepare(dev-clk_per); +err_clk_per: v4l2_fh_del(ctx-fh); v4l2_fh_exit(ctx-fh); clear_bit(ctx-idx, dev-instance_mask); -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/3] [media] coda: Fix error paths
Some resources were not being released in the error path and some were released
in the incorrect order.
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
Changes since v3:
- Only disable the clocks after v4l2_m2m_ctx_release()
Changes since v2:
- Newly introduced in this series
drivers/media/platform/coda.c | 38 --
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index df4ada88..9384f02 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -1514,21 +1514,26 @@ static int coda_open(struct file *file)
int ret = 0;
int idx;
- idx = coda_next_free_instance(dev);
- if (idx = CODA_MAX_INSTANCES)
- return -EBUSY;
- set_bit(idx, dev-instance_mask);
-
ctx = kzalloc(sizeof *ctx, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
+ idx = coda_next_free_instance(dev);
+ if (idx = CODA_MAX_INSTANCES) {
+ ret = -EBUSY;
+ goto err_coda_max;
+ }
+ set_bit(idx, dev-instance_mask);
+
v4l2_fh_init(ctx-fh, video_devdata(file));
file-private_data = ctx-fh;
v4l2_fh_add(ctx-fh);
ctx-dev = dev;
ctx-idx = idx;
+ clk_prepare_enable(dev-clk_per);
+ clk_prepare_enable(dev-clk_ahb);
+
set_default_params(ctx);
ctx-m2m_ctx = v4l2_m2m_ctx_init(dev-m2m_dev, ctx,
coda_queue_init);
@@ -1537,12 +1542,12 @@ static int coda_open(struct file *file)
v4l2_err(dev-v4l2_dev, %s return error (%d)\n,
__func__, ret);
- goto err;
+ goto err_ctx_init;
}
ret = coda_ctrls_setup(ctx);
if (ret) {
v4l2_err(dev-v4l2_dev, failed to setup coda controls\n);
- goto err;
+ goto err_ctrls_setup;
}
ctx-fh.ctrl_handler = ctx-ctrls;
@@ -1552,24 +1557,29 @@ static int coda_open(struct file *file)
if (!ctx-parabuf.vaddr) {
v4l2_err(dev-v4l2_dev, failed to allocate parabuf);
ret = -ENOMEM;
- goto err;
+ goto err_dma_alloc;
}
coda_lock(ctx);
list_add(ctx-list, dev-instances);
coda_unlock(ctx);
- clk_prepare_enable(dev-clk_per);
- clk_prepare_enable(dev-clk_ahb);
-
v4l2_dbg(1, coda_debug, dev-v4l2_dev, Created instance %d (%p)\n,
ctx-idx, ctx);
return 0;
-err:
+err_dma_alloc:
+ v4l2_ctrl_handler_free(ctx-ctrls);
+err_ctrls_setup:
+ v4l2_m2m_ctx_release(ctx-m2m_ctx);
+err_ctx_init:
+ clk_disable_unprepare(dev-clk_ahb);
+ clk_disable_unprepare(dev-clk_per);
v4l2_fh_del(ctx-fh);
v4l2_fh_exit(ctx-fh);
+ clear_bit(ctx-idx, dev-instance_mask);
+err_coda_max:
kfree(ctx);
return ret;
}
@@ -1588,10 +1598,10 @@ static int coda_release(struct file *file)
dma_free_coherent(dev-plat_dev-dev, CODA_PARA_BUF_SIZE,
ctx-parabuf.vaddr, ctx-parabuf.paddr);
- v4l2_m2m_ctx_release(ctx-m2m_ctx);
v4l2_ctrl_handler_free(ctx-ctrls);
- clk_disable_unprepare(dev-clk_per);
+ v4l2_m2m_ctx_release(ctx-m2m_ctx);
clk_disable_unprepare(dev-clk_ahb);
+ clk_disable_unprepare(dev-clk_per);
v4l2_fh_del(ctx-fh);
v4l2_fh_exit(ctx-fh);
clear_bit(ctx-idx, dev-instance_mask);
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/3] [media] coda: No need to check the return value of platform_get_resource()
When using devm_ioremap_resource(), we do not need to check the return value of
platform_get_resource(), so just remove it.
Signed-off-by: Fabio Estevam fabio.este...@freescale.com
Acked-by: Philipp Zabel p.za...@pengutronix.de
---
Changes since v3:
- None
Changes since v2:
- None
Changes since v1:
- None
drivers/media/platform/coda.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 2d1576b..4a0a421 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -2049,11 +2049,6 @@ static int coda_probe(struct platform_device *pdev)
/* Get memory for physical registers */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res == NULL) {
- dev_err(pdev-dev, failed to get memory region resource\n);
- return -ENOENT;
- }
-
dev-regs_base = devm_ioremap_resource(pdev-dev, res);
if (IS_ERR(dev-regs_base))
return PTR_ERR(dev-regs_base);
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself? If
you
had a priv pointer to search from, then you could have just passed
the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but let's
try to check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are
really helpful.
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
---
-8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data structure
unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call to
phy_create(). Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).
Hmm? So, do you suggest to call phy_create() from board file? What phy_ops
struct and other hardware parameters would it take?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in some,
not so uncommon cases, but there is already a lot of existing lookup
solutions over the kernel and so there is no point in introducing
another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.
There was a reason for introducing lookup solutions. The reason was that in
board file there is no way to get a pointer to something that is going to be
created much later in time. We don't do time travel ;-).
Please just pass the real phy pointer around, that's what it is
there
for. Your board binding logic/code should be able to handle this,
as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't really
nice, because it's a layering violation (at least if I understood what
you mean). This is because you need to have full definition of struct
phy in board file and a structure that is used as private data in PHY
core comes from platform code.
No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.
Does that make more sense?
Well, to the point that I think I now understood your suggestion.
Unfortunately the suggestion alone isn't really something that can be done,
considering how driver core and generic frameworks work.
Ok, given that I seem to be totally confused as to exactly how the
board-specific frameworks work, I'll take your word for it.
But again, I will not accept lookup by name type solutions, when the
name is dynamic and will change. Because you are using a name, you
can deal with a pointer, putting it _somewhere_ in your board-specific
data structures, as you are going to need to store it anyway (hint, you
had to get that name from somewhere, right?)
And maybe the way that these generic frameworks are created is wrong,
given that you don't feel that a generic pointer can be passed to the
needed devices. That seems like a huge problem, one that has already
been pointed out is causing issues with other subsystems.
So maybe they need to be fixed?
thanks,
greg k-h
--
To
[REVIEW PATCH 0/6] exynos4-is: Asynchronous subdev registration support
This patch series is a refactoring of the exynos4-is driver to get rid of the common fimc-is-sensor driver and to adapt it to use standard sensor subdev drivers, one per each image sensor type. Then a clock provider is added to the exynos4-is driver and the s5k6a3 subdev is modified to use one of the clocks registered by exynos4-is. Arun, I think you could reuse the s5k6a3 sensor for your work on the Exynos5 FIMC-IS driver. One advantage of separate sensor drivers is that the power on/off sequences can be written specifically for each sensor. We are probably going to need such sequences per board in future. Also having the clock control inside the sensor subdev allows to better match the hardware power on/off sequence requirements, however the S5K6A3 sensor can have active clock signal on its clock input pin even when all its power supplies are turned off. I'm posting this series before having a proper implementation for clk_unregister() in the clock framework, so you are not blocked with your Exynos5 FIMC-IS works. This series with all dependencies can be found at: http://git.linuxtv.org/snawrocki/samsung.git/exynos4-is-clk Thanks, Sylwester Sylwester Nawrocki (6): V4L: Add driver for s5k6a3 image sensor V4L: s5k6a3: Add support for asynchronous subdev registration exynos4-is: Simplify sclk_cam clocks handling exynos4-is: Add clock provider for the external clocks exynos4-is: Use external s5k6a3 sensor driver exynos4-is: Add support for asynchronous sensor subddevs registration .../devicetree/bindings/media/samsung-fimc.txt | 17 +- drivers/media/i2c/Kconfig |8 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k6a3.c | 356 drivers/media/platform/exynos4-is/fimc-is-regs.c |2 +- drivers/media/platform/exynos4-is/fimc-is-sensor.c | 285 +--- drivers/media/platform/exynos4-is/fimc-is-sensor.h | 49 +-- drivers/media/platform/exynos4-is/fimc-is.c| 96 +++--- drivers/media/platform/exynos4-is/fimc-is.h|4 +- drivers/media/platform/exynos4-is/media-dev.c | 268 ++- drivers/media/platform/exynos4-is/media-dev.h | 31 +- 11 files changed, 650 insertions(+), 467 deletions(-) create mode 100644 drivers/media/i2c/s5k6a3.c -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 1/6] V4L: Add driver for s5k6a3 image sensor
This patch adds subdev driver for Samsung S5K6A3 raw image sensor.
As it is intended at the moment to be used only with the Exynos
FIMC-IS (camera ISP) subsystem it is pretty minimal subdev driver.
It doesn't do any I2C communication since the sensor is controlled
by the ISP and its own firmware.
This driver can be updated in future, should anyone need it to be
a regular subdev driver where the main CPU communicates with the
sensor directly.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/i2c/Kconfig |8 ++
drivers/media/i2c/Makefile |1 +
drivers/media/i2c/s5k6a3.c | 315
3 files changed, 324 insertions(+)
create mode 100644 drivers/media/i2c/s5k6a3.c
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 3367ec2..dbd9cbb 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -564,6 +564,14 @@ config VIDEO_S5K6AA
This is a V4L2 sensor-level driver for Samsung S5K6AA(FX) 1.3M
camera sensor with an embedded SoC image signal processor.
+config VIDEO_S5K6A3
+ tristate Samsung S5K6A3 sensor support
+ depends on MEDIA_CAMERA_SUPPORT
+ depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API OF
+ ---help---
+ This is a V4L2 sensor-level driver for Samsung S5K6A3 raw
+ camera sensor.
+
config VIDEO_S5K4ECGX
tristate Samsung S5K4ECGX sensor support
depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d590925..44998a2 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_VIDEO_MT9V032) += mt9v032.o
obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
+obj-$(CONFIG_VIDEO_S5K6A3) += s5k6a3.o
obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o
obj-$(CONFIG_VIDEO_S5K5BAF)+= s5k5baf.o
obj-$(CONFIG_VIDEO_S5C73M3)+= s5c73m3/
diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
new file mode 100644
index 000..21680fa
--- /dev/null
+++ b/drivers/media/i2c/s5k6a3.c
@@ -0,0 +1,315 @@
+/*
+ * Samsung S5K6A3 image sensor driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki s.nawro...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/errno.h
+#include linux/gpio.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of_gpio.h
+#include linux/pm_runtime.h
+#include linux/regulator/consumer.h
+#include linux/slab.h
+#include linux/videodev2.h
+#include media/v4l2-async.h
+#include media/v4l2-subdev.h
+
+#define S5K6A3_SENSOR_MAX_WIDTH1392
+#define S5K6A3_SENSOR_MAX_HEIGHT 1392
+#define S5K6A3_SENSOR_MIN_WIDTH32
+#define S5K6A3_SENSOR_MIN_HEIGHT 32
+
+#define S5K6A3_DEF_PIX_WIDTH 1296
+#define S5K6A3_DEF_PIX_HEIGHT 732
+
+#define S5K6A3_DRV_NAMES5K6A3
+
+#define S5K6A3_NUM_SUPPLIES2
+
+/**
+ * struct s5k6a3 - fimc-is sensor data structure
+ * @dev: pointer to this I2C client device structure
+ * @subdev: the image sensor's v4l2 subdev
+ * @pad: subdev media source pad
+ * @supplies: image sensor's voltage regulator supplies
+ * @gpio_reset: GPIO connected to the sensor's reset pin
+ * @lock: mutex protecting the structure's members below
+ * @format: media bus format at the sensor's source pad
+ */
+struct s5k6a3 {
+ struct device *dev;
+ struct v4l2_subdev subdev;
+ struct media_pad pad;
+ struct regulator_bulk_data supplies[S5K6A3_NUM_SUPPLIES];
+ int gpio_reset;
+ struct mutex lock;
+ struct v4l2_mbus_framefmt format;
+ u32 clock_frequency;
+};
+
+static const char * const s5k6a3_supply_names[] = {
+ svdda,
+ svddio
+};
+
+static inline struct s5k6a3 *sd_to_s5k6a3(struct v4l2_subdev *sd)
+{
+ return container_of(sd, struct s5k6a3, subdev);
+}
+
+static const struct v4l2_mbus_framefmt s5k6a3_formats[] = {
+ {
+ .code = V4L2_MBUS_FMT_SGRBG10_1X10,
+ .colorspace = V4L2_COLORSPACE_SRGB,
+ .field = V4L2_FIELD_NONE,
+ }
+};
+
+static const struct v4l2_mbus_framefmt *find_sensor_format(
+ struct v4l2_mbus_framefmt *mf)
+{
+ int i;
+
+ for (i = 0; i ARRAY_SIZE(s5k6a3_formats); i++)
+ if (mf-code == s5k6a3_formats[i].code)
+ return s5k6a3_formats[i];
+
+ return s5k6a3_formats[0];
+}
+
+static int s5k6a3_enum_mbus_code(struct v4l2_subdev *sd,
+
[REVIEW PATCH 2/6] V4L: s5k6a3: Add support for asynchronous subdev registration
This patch converts the driver to use v4l2 asynchronous subdev
registration API an the common clock API.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/i2c/s5k6a3.c | 63
1 file changed, 52 insertions(+), 11 deletions(-)
diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
index 21680fa..ccbb4fc 100644
--- a/drivers/media/i2c/s5k6a3.c
+++ b/drivers/media/i2c/s5k6a3.c
@@ -34,6 +34,7 @@
#define S5K6A3_DEF_PIX_HEIGHT 732
#define S5K6A3_DRV_NAMES5K6A3
+#define S5K6A3_CLK_NAMEmclk
#define S5K6A3_NUM_SUPPLIES2
@@ -55,6 +56,7 @@ struct s5k6a3 {
int gpio_reset;
struct mutex lock;
struct v4l2_mbus_framefmt format;
+ struct clk *clock;
u32 clock_frequency;
};
@@ -180,19 +182,29 @@ static int s5k6a3_s_power(struct v4l2_subdev *sd, int on)
{
struct s5k6a3 *sensor = sd_to_s5k6a3(sd);
int gpio = sensor-gpio_reset;
- int ret;
+ int ret = 0;
if (on) {
+ sensor-clock = clk_get(sensor-dev, S5K6A3_CLK_NAME);
+ if (IS_ERR(sensor-clock))
+ return PTR_ERR(sensor-clock);
+
+ ret = clk_set_rate(sensor-clock, sensor-clock_frequency);
+ if (ret 0)
+ goto clk_put;
+
ret = pm_runtime_get(sensor-dev);
if (ret 0)
- return ret;
+ goto clk_put;
ret = regulator_bulk_enable(S5K6A3_NUM_SUPPLIES,
sensor-supplies);
- if (ret 0) {
- pm_runtime_put(sensor-dev);
- return ret;
- }
+ if (ret 0)
+ goto rpm_put;
+
+ ret = clk_prepare_enable(sensor-clock);
+ if (ret 0)
+ goto reg_dis;
if (gpio_is_valid(gpio)) {
gpio_set_value(gpio, 1);
@@ -208,10 +220,14 @@ static int s5k6a3_s_power(struct v4l2_subdev *sd, int on)
if (gpio_is_valid(gpio))
gpio_set_value(gpio, 0);
- ret = regulator_bulk_disable(S5K6A3_NUM_SUPPLIES,
-sensor-supplies);
- if (!ret)
- pm_runtime_put(sensor-dev);
+ clk_disable_unprepare(sensor-clock);
+reg_dis:
+ regulator_bulk_disable(S5K6A3_NUM_SUPPLIES,
+ sensor-supplies);
+rpm_put:
+ pm_runtime_put(sensor-dev);
+clk_put:
+ clk_put(sensor-clock);
}
return ret;
}
@@ -239,6 +255,7 @@ static int s5k6a3_probe(struct i2c_client *client,
mutex_init(sensor-lock);
sensor-gpio_reset = -EINVAL;
+ sensor-clock = ERR_PTR(-EINVAL);
sensor-dev = dev;
gpio = of_get_gpio_flags(dev-of_node, 0, NULL);
@@ -250,6 +267,13 @@ static int s5k6a3_probe(struct i2c_client *client,
}
sensor-gpio_reset = gpio;
+ if (of_property_read_u32(dev-of_node, clock-frequency,
+sensor-clock_frequency)) {
+ dev_err(dev, clock-frequency property not found at %s\n,
+ dev-of_node-full_name);
+ return -EINVAL;
+ }
+
for (i = 0; i S5K6A3_NUM_SUPPLIES; i++)
sensor-supplies[i].supply = s5k6a3_supply_names[i];
@@ -258,6 +282,11 @@ static int s5k6a3_probe(struct i2c_client *client,
if (ret 0)
return ret;
+ /* Defer probing if the clock is not available yet */
+ sensor-clock = clk_get(dev, S5K6A3_CLK_NAME);
+ if (IS_ERR(sensor-clock))
+ return -EPROBE_DEFER;
+
sd = sensor-subdev;
v4l2_i2c_subdev_init(sd, client, s5k6a3_subdev_ops);
snprintf(sd-name, sizeof(sd-name), S5K6A3_DRV_NAME);
@@ -275,12 +304,24 @@ static int s5k6a3_probe(struct i2c_client *client,
pm_runtime_no_callbacks(dev);
pm_runtime_enable(dev);
- return 0;
+ ret = v4l2_async_register_subdev(sd);
+
+ /*
+* Don't hold reference to the clock to avoid circular dependency
+* between the subdev and the host driver, in case the host is
+* a supplier of the clock.
+* clk_get()/clk_put() will be called in s_power callback.
+*/
+ clk_put(sensor-clock);
+
+ return ret;
}
static int s5k6a3_remove(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
+
+ v4l2_async_unregister_subdev(sd);
media_entity_cleanup(sd-entity);
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
[REVIEW PATCH 3/6] exynos4-is: Simplify sclk_cam clocks handling
Use clk_prepare_enable()/clk_disable_unprepare() instead of
separately prearing/unparing the clk_cam clocks. This simplifies
the code that is now mostly not going to be used, function
__fimc_md_set_camclk() is only left for S5PV210 platform which
is not yet converted to Device Tree.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/exynos4-is/media-dev.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c
b/drivers/media/platform/exynos4-is/media-dev.c
index 91f21e2..41366fe 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1150,7 +1150,6 @@ static void fimc_md_put_clocks(struct fimc_md *fmd)
while (--i = 0) {
if (IS_ERR(fmd-camclk[i].clock))
continue;
- clk_unprepare(fmd-camclk[i].clock);
clk_put(fmd-camclk[i].clock);
fmd-camclk[i].clock = ERR_PTR(-EINVAL);
}
@@ -1169,7 +1168,7 @@ static int fimc_md_get_clocks(struct fimc_md *fmd)
struct device *dev = NULL;
char clk_name[32];
struct clk *clock;
- int ret, i;
+ int i, ret = 0;
for (i = 0; i FIMC_MAX_CAMCLKS; i++)
fmd-camclk[i].clock = ERR_PTR(-EINVAL);
@@ -1187,12 +1186,6 @@ static int fimc_md_get_clocks(struct fimc_md *fmd)
ret = PTR_ERR(clock);
break;
}
- ret = clk_prepare(clock);
- if (ret 0) {
- clk_put(clock);
- fmd-camclk[i].clock = ERR_PTR(-EINVAL);
- break;
- }
fmd-camclk[i].clock = clock;
}
if (ret)
@@ -1249,7 +1242,7 @@ static int __fimc_md_set_camclk(struct fimc_md *fmd,
ret = pm_runtime_get_sync(fmd-pmf);
if (ret 0)
return ret;
- ret = clk_enable(camclk-clock);
+ ret = clk_prepare_enable(camclk-clock);
dbg(Enabled camclk %d: f: %lu, si-clk_id,
clk_get_rate(camclk-clock));
}
@@ -1260,7 +1253,7 @@ static int __fimc_md_set_camclk(struct fimc_md *fmd,
return 0;
if (--camclk-use_count == 0) {
- clk_disable(camclk-clock);
+ clk_disable_unprepare(camclk-clock);
pm_runtime_put(fmd-pmf);
dbg(Disabled camclk %d, si-clk_id);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEW PATCH 4/6] exynos4-is: Add clock provider for the external clocks
This patch adds clock provider to expose the sclk_cam0/1 clocks
for image sensor subdevs.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../devicetree/bindings/media/samsung-fimc.txt | 17 +++-
drivers/media/platform/exynos4-is/media-dev.c | 92
drivers/media/platform/exynos4-is/media-dev.h | 19 +++-
3 files changed, 125 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt
b/Documentation/devicetree/bindings/media/samsung-fimc.txt
index 96312f6..04a2b87 100644
--- a/Documentation/devicetree/bindings/media/samsung-fimc.txt
+++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt
@@ -91,6 +91,15 @@ Optional properties
- samsung,camclk-out : specifies clock output for remote sensor,
0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT;
+'clock-controller' node (optional)
+--
+
+The purpose of this node is to define a clock provider for external image
+sensors and link any of the CAM_?_CLKOUT clock outputs with related external
+clock consumer device. Properties specific to this node are described in
+../clock/clock-bindings.txt.
+
+
Image sensor nodes
--
@@ -114,7 +123,7 @@ Example:
vddio-supply = ...;
clock-frequency = 2400;
- clocks = ...;
+ clocks = camclk 1;
clock-names = mclk;
port {
@@ -135,7 +144,7 @@ Example:
vddio-supply = ...;
clock-frequency = 2400;
- clocks = ...;
+ clocks = camclk 0;
clock-names = mclk;
port {
@@ -156,6 +165,10 @@ Example:
pinctrl-names = default;
pinctrl-0 = cam_port_a_clk_active;
+ camclk: clock-controller {
+ #clock-cells = 1;
+ };
+
/* parallel camera ports */
parallel-ports {
/* camera A input */
diff --git a/drivers/media/platform/exynos4-is/media-dev.c
b/drivers/media/platform/exynos4-is/media-dev.c
index 41366fe..346e1e0 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -11,6 +11,8 @@
*/
#include linux/bug.h
+#include linux/clk.h
+#include linux/clk-provider.h
#include linux/device.h
#include linux/errno.h
#include linux/i2c.h
@@ -1438,6 +1440,86 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)
return 0;
}
+#ifdef CONFIG_OF
+struct cam_clk {
+ struct clk_hw hw;
+ struct fimc_md *fmd;
+};
+#define to_cam_clk(_hw) container_of(_hw, struct cam_clk, hw)
+
+static int cam_clk_prepare(struct clk_hw *hw)
+{
+ struct cam_clk *camclk = to_cam_clk(hw);
+ int ret = pm_runtime_get_sync(camclk-fmd-pmf);
+
+ return ret 0 ? ret : 0;
+}
+
+static void cam_clk_unprepare(struct clk_hw *hw)
+{
+ struct cam_clk *camclk = to_cam_clk(hw);
+ pm_runtime_put_sync(camclk-fmd-pmf);
+}
+
+static const struct clk_ops cam_clk_ops = {
+ .prepare = cam_clk_prepare,
+ .unprepare = cam_clk_unprepare,
+};
+
+static const char *cam_clk_p_names[] = { sclk_cam0, sclk_cam1 };
+
+static int fimc_md_register_clk_provider(struct fimc_md *fmd)
+{
+ struct cam_clk_provider *clk_provider = fmd-clk_provider;
+ struct device *dev = fmd-pdev-dev;
+ struct device_node *node;
+ unsigned int nclocks;
+
+ node = of_get_child_by_name(dev-of_node, clock-controller);
+ if (!node) {
+ dev_warn(dev, clock-controller node at %s not found\n,
+ dev-of_node-full_name);
+ return 0;
+ }
+ /* Instantiate the clocks */
+ for (nclocks = 0; nclocks FIMC_MAX_CAMCLKS; nclocks++) {
+ struct clk_init_data init;
+ char clk_name[16];
+ struct clk *clk;
+ struct cam_clk *camclk;
+
+ camclk = devm_kzalloc(dev, sizeof(*camclk), GFP_KERNEL);
+ if (!camclk)
+ return -ENOMEM;
+
+ snprintf(clk_name, sizeof(clk_name), cam_clkout%d, nclocks);
+
+ init.name = clk_name;
+ init.ops = cam_clk_ops;
+ init.flags = CLK_SET_RATE_PARENT;
+ init.parent_names = cam_clk_p_names[nclocks];
+ init.num_parents = 1;
+ camclk-hw.init = init;
+ camclk-fmd = fmd;
+
+ clk = devm_clk_register(dev, camclk-hw);
+ if (IS_ERR(clk)) {
+ kfree(camclk);
+ return PTR_ERR(clk);
+ }
+ clk_provider-clks[nclocks] = clk;
+ }
+
+
[REVIEW PATCH 6/6] exynos4-is: Add support for asynchronous sensor subddevs registration
Add support registering external sensor subdevs using the v4l2-async API.
The async API is used only for sensor subdevs and only for platforms
instatiated from Device Tree.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/exynos4-is/media-dev.c | 163 ++---
drivers/media/platform/exynos4-is/media-dev.h | 12 +-
2 files changed, 100 insertions(+), 75 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c
b/drivers/media/platform/exynos4-is/media-dev.c
index 346e1e0..280e819 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -27,6 +27,7 @@
#include linux/pm_runtime.h
#include linux/types.h
#include linux/slab.h
+#include media/v4l2-async.h
#include media/v4l2-ctrls.h
#include media/v4l2-of.h
#include media/media-device.h
@@ -221,6 +222,7 @@ static int __fimc_pipeline_open(struct
exynos_media_pipeline *ep,
if (ret 0)
return ret;
}
+
ret = fimc_md_set_camclk(sd, true);
if (ret 0)
goto err_wbclk;
@@ -395,63 +397,6 @@ static void fimc_md_unregister_sensor(struct v4l2_subdev
*sd)
}
#ifdef CONFIG_OF
-/* Register I2C client subdev associated with @node. */
-static int fimc_md_of_add_sensor(struct fimc_md *fmd,
-struct device_node *node, int index)
-{
- struct fimc_sensor_info *si;
- struct i2c_client *client;
- struct v4l2_subdev *sd;
- int ret;
-
- if (WARN_ON(index = ARRAY_SIZE(fmd-sensor)))
- return -EINVAL;
- si = fmd-sensor[index];
-
- client = of_find_i2c_device_by_node(node);
- if (!client)
- return -EPROBE_DEFER;
-
- device_lock(client-dev);
-
- if (!client-driver ||
- !try_module_get(client-driver-driver.owner)) {
- ret = -EPROBE_DEFER;
- v4l2_info(fmd-v4l2_dev, No driver found for %s\n,
- node-full_name);
- goto dev_put;
- }
-
- /* Enable sensor's master clock */
- ret = __fimc_md_set_camclk(fmd, si-pdata, true);
- if (ret 0)
- goto mod_put;
- sd = i2c_get_clientdata(client);
-
- ret = v4l2_device_register_subdev(fmd-v4l2_dev, sd);
- __fimc_md_set_camclk(fmd, si-pdata, false);
- if (ret 0)
- goto mod_put;
-
- v4l2_set_subdev_hostdata(sd, si-pdata);
- if (si-pdata.fimc_bus_type == FIMC_BUS_TYPE_ISP_WRITEBACK)
- sd-grp_id = GRP_ID_FIMC_IS_SENSOR;
- else
- sd-grp_id = GRP_ID_SENSOR;
-
- si-subdev = sd;
- v4l2_info(fmd-v4l2_dev, Registered sensor subdevice: %s (%d)\n,
- sd-name, fmd-num_sensors);
- fmd-num_sensors++;
-
-mod_put:
- module_put(client-driver-driver.owner);
-dev_put:
- device_unlock(client-dev);
- put_device(client-dev);
- return ret;
-}
-
/* Parse port node and register as a sub-device any sensor specified there. */
static int fimc_md_parse_port_node(struct fimc_md *fmd,
struct device_node *port,
@@ -460,7 +405,6 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
struct device_node *rem, *ep, *np;
struct fimc_source_info *pd;
struct v4l2_of_endpoint endpoint;
- int ret;
u32 val;
pd = fmd-sensor[index].pdata;
@@ -527,10 +471,17 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
else
pd-fimc_bus_type = pd-sensor_bus_type;
- ret = fimc_md_of_add_sensor(fmd, rem, index);
- of_node_put(rem);
+ if (WARN_ON(index = ARRAY_SIZE(fmd-sensor)))
+ return -EINVAL;
- return ret;
+ fmd-sensor[index].asd.match_type = V4L2_ASYNC_MATCH_OF;
+ fmd-sensor[index].asd.match.of.node = rem;
+ fmd-async_subdevs[index] = fmd-sensor[index].asd;
+
+ fmd-num_sensors++;
+
+ of_node_put(rem);
+ return 0;
}
/* Register all SoC external sub-devices */
@@ -1225,6 +1176,14 @@ static int __fimc_md_set_camclk(struct fimc_md *fmd,
struct fimc_camclk_info *camclk;
int ret = 0;
+ /*
+* When device tree is used the sensor drivers are supposed to
+* control the clock themselves. This whole function will be
+* removed once S5PV210 platform is converted to the device tree.
+*/
+ if (fmd-pdev-dev.of_node)
+ return 0;
+
if (WARN_ON(si-clk_id = FIMC_MAX_CAMCLKS) || !fmd || !fmd-pmf)
return -EINVAL;
@@ -1520,6 +1479,56 @@ static int fimc_md_register_clk_provider(struct fimc_md
*fmd)
#define fimc_md_register_clk_provider(fmd) (0)
#endif
+static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
+struct v4l2_subdev
[REVIEW PATCH 5/6] exynos4-is: Use external s5k6a3 sensor driver
This patch removes the common fimc-is-sensor driver for image sensors
that are normally controlled by the FIMC-IS firmware. The FIMC-IS
driver now contains only a table of properties specific to each sensor.
The sensor properties required for the ISP's firmware are parsed from
device tree and retrieved from the internal table, which is selected
based on the compatible property of an image sensor.
To use the Exynos4x12 internal ISP the S5K6A3 sensor driver (drivers/
media/i2c/s5k6a3.c) is now required.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/media/platform/exynos4-is/fimc-is-regs.c |2 +-
drivers/media/platform/exynos4-is/fimc-is-sensor.c | 285 +---
drivers/media/platform/exynos4-is/fimc-is-sensor.h | 49 +---
drivers/media/platform/exynos4-is/fimc-is.c| 96 +++
drivers/media/platform/exynos4-is/fimc-is.h|4 +-
5 files changed, 57 insertions(+), 379 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/fimc-is-regs.c
b/drivers/media/platform/exynos4-is/fimc-is-regs.c
index 63c68ec..3e51aa6 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-regs.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-regs.c
@@ -136,7 +136,7 @@ void fimc_is_hw_set_sensor_num(struct fimc_is *is)
mcuctl_write(IH_REPLY_DONE, is, MCUCTL_REG_ISSR(0));
mcuctl_write(is-sensor_index, is, MCUCTL_REG_ISSR(1));
mcuctl_write(IHC_GET_SENSOR_NUM, is, MCUCTL_REG_ISSR(2));
- mcuctl_write(FIMC_IS_SENSOR_NUM, is, MCUCTL_REG_ISSR(3));
+ mcuctl_write(FIMC_IS_SENSORS_NUM, is, MCUCTL_REG_ISSR(3));
}
void fimc_is_hw_close_sensor(struct fimc_is *is, unsigned int index)
diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c
b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
index 6647421..10e82e2 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c
@@ -2,276 +2,21 @@
* Samsung EXYNOS4x12 FIMC-IS (Imaging Subsystem) driver
*
* Copyright (C) 2013 Samsung Electronics Co., Ltd.
- *
* Author: Sylwester Nawrocki s.nawro...@samsung.com
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
-#include linux/delay.h
-#include linux/device.h
-#include linux/errno.h
-#include linux/gpio.h
-#include linux/i2c.h
-#include linux/kernel.h
-#include linux/module.h
-#include linux/of_gpio.h
-#include linux/pm_runtime.h
-#include linux/regulator/consumer.h
-#include linux/slab.h
-#include media/v4l2-subdev.h
-#include fimc-is.h
#include fimc-is-sensor.h
-#define DRIVER_NAME FIMC-IS-SENSOR
-
-static const char * const sensor_supply_names[] = {
- svdda,
- svddio,
-};
-
-static const struct v4l2_mbus_framefmt fimc_is_sensor_formats[] = {
- {
- .code = V4L2_MBUS_FMT_SGRBG10_1X10,
- .colorspace = V4L2_COLORSPACE_SRGB,
- .field = V4L2_FIELD_NONE,
- }
-};
-
-static const struct v4l2_mbus_framefmt *find_sensor_format(
- struct v4l2_mbus_framefmt *mf)
-{
- int i;
-
- for (i = 0; i ARRAY_SIZE(fimc_is_sensor_formats); i++)
- if (mf-code == fimc_is_sensor_formats[i].code)
- return fimc_is_sensor_formats[i];
-
- return fimc_is_sensor_formats[0];
-}
-
-static int fimc_is_sensor_enum_mbus_code(struct v4l2_subdev *sd,
- struct v4l2_subdev_fh *fh,
- struct v4l2_subdev_mbus_code_enum *code)
-{
- if (code-index = ARRAY_SIZE(fimc_is_sensor_formats))
- return -EINVAL;
-
- code-code = fimc_is_sensor_formats[code-index].code;
- return 0;
-}
-
-static void fimc_is_sensor_try_format(struct fimc_is_sensor *sensor,
- struct v4l2_mbus_framefmt *mf)
-{
- const struct sensor_drv_data *dd = sensor-drvdata;
- const struct v4l2_mbus_framefmt *fmt;
-
- fmt = find_sensor_format(mf);
- mf-code = fmt-code;
- v4l_bound_align_image(mf-width, 16 + 8, dd-width, 0,
- mf-height, 12 + 8, dd-height, 0, 0);
-}
-
-static struct v4l2_mbus_framefmt *__fimc_is_sensor_get_format(
- struct fimc_is_sensor *sensor, struct v4l2_subdev_fh *fh,
- u32 pad, enum v4l2_subdev_format_whence which)
-{
- if (which == V4L2_SUBDEV_FORMAT_TRY)
- return fh ? v4l2_subdev_get_try_format(fh, pad) : NULL;
-
- return sensor-format;
-}
-
-static int fimc_is_sensor_set_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_fh *fh,
- struct v4l2_subdev_format *fmt)
-{
- struct fimc_is_sensor *sensor = sd_to_fimc_is_sensor(sd);
- struct v4l2_mbus_framefmt *mf;
-
-
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 11:01:10AM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 06:44:56PM +0100, Mark Brown wrote: What are the problems you are seeing with doing things with lookups? You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) I got copied in very late so don't have most of the thread I'm afraid, I did try looking at web archives but didn't see a clear problem statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... It's adding platform data in the first place that gets tedious - and of course there's also DT and ACPI to worry about, it's not just a case of platform data and then you're done. Pushing the lookup into library code means that drivers don't have to worry about any of this stuff. For most of the APIs doing this there is a clear and unambiguous name in the hardware that can be used (and for hardware process reasons is unlikely to get changed). The major exception to this is the clock API since it is relatively rare to have clear, segregated IP level information for IPs baked into larger chips. The other APIs tend to be establishing chip to chip links. signature.asc Description: Digital signature
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote:
IMHO it would be better if you provided some code example, but let's try to
check if I understood you correctly.
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
This should be controller_pdev, not phy_pdev, yes?
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
Or even just phy_get(pdev-dev), because phy_get() could be smart
enough to to set phy = dev-platform_data.
8
Is this what you mean?
That's what I was going to suggest too. The struct phy is defined in
the board file, which already knows about all the PHYs that exist in
the system. (Or perhaps it is allocated dynamically, so that when many
board files are present in the same kernel, only the entries listed in
the board file for the current system get created.) Then the
structure's address is stored in the platform data and made available
to both the provider and the consumer.
Even though the struct phy is defined (or allocated) in the board file,
its contents don't get filled in until the PHY driver provides the
details.
It's technically correct, but quality of this solution isn't really nice,
because it's a layering violation (at least if I understood what you mean).
This is because you need to have full definition of struct phy in board file
and a structure that is used as private data in PHY core comes from
platform code.
You don't have to have a full definition in the board file. Just a
partial definition -- most of the contents can be filled in later, when
the PHY driver is ready to store the private data.
It's not a layering violation for one region of the kernel to store
private data in a structure defined by another part of the kernel.
This happens all the time (e.g., dev_set_drvdata).
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote: You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) I got copied in very late so don't have most of the thread I'm afraid, I did try looking at web archives but didn't see a clear problem statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. What do you mean by locally? The problem with the api was that the phy core wanted a id and a name to create a phy, and then later other code was doing a lookup based on the name and id (mushed together), because it knew that this device was the one it wanted. Just like the clock api, which, for multiple devices, has proven to cause problems. I don't want to see us accept an api that we know has issues in it now, I'd rather us fix it up properly. Subsystems should be able to create ids how ever they want to, and not rely on the code calling them to specify the names of the devices that way, otherwise the api is just too fragile. I think, that if you create a device, then just carry around the pointer to that device (in this case a phy) and pass it to whatever other code needs it. No need to do lookups on known names or anything else, just normal pointers, with no problems for multiple devices, busses, or naming issues. Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... It's adding platform data in the first place that gets tedious - and of course there's also DT and ACPI to worry about, it's not just a case of platform data and then you're done. Pushing the lookup into library code means that drivers don't have to worry about any of this stuff. I agree, so just pass around the pointer to the phy and all is good. No need to worry about DT or ACPI or anything else. For most of the APIs doing this there is a clear and unambiguous name in the hardware that can be used (and for hardware process reasons is unlikely to get changed). The major exception to this is the clock API since it is relatively rare to have clear, segregated IP level information for IPs baked into larger chips. The other APIs tend to be establishing chip to chip links. The clock api is having problems with multiple names due to dynamic devices from what I was told. I want to prevent the PHY interface from having that same issue. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier
in
this thread for details about that.)
I got copied in very late so don't have most of the thread I'm afraid,
I did try looking at web archives but didn't see a clear problem
statement. In any case this is why the APIs doing lookups do the
lookups in the context of the requesting device - devices ask for
whatever name they use locally.
What do you mean by locally?
The problem with the api was that the phy core wanted a id and a name to
create a phy, and then later other code was doing a lookup based on
the name and id (mushed together), because it knew that this device
was the one it wanted.
Just like the clock api, which, for multiple devices, has proven to
cause problems. I don't want to see us accept an api that we know has
issues in it now, I'd rather us fix it up properly.
Subsystems should be able to create ids how ever they want to, and not
rely on the code calling them to specify the names of the devices that
way, otherwise the api is just too fragile.
I think, that if you create a device, then just carry around the pointer
to that device (in this case a phy) and pass it to whatever other code
needs it. No need to do lookups on known names or anything else,
just normal pointers, with no problems for multiple devices, busses, or
naming issues.
PHY object is not a device, it is something that a device driver creates
(one or more instances of) when it is being probed. You don't have a clean
way to export this PHY object to other driver, other than keeping this PHY
on a list inside PHY core with some well-known ID (e.g. device name +
consumer port name/index, like in regulator core) and then to use this
well-known ID inside consumer driver as a lookup key passed to phy_get();
Actually I think for PHY case, exactly the same way as used for regulators
might be completely fine:
1. Each PHY would have some kind of platform, non-unique name, that is
just used to print some messages (like the platform/board name of a
regulator).
2. Each PHY would have an array of consumers. Consumer specifier would
consist of consumer device name and consumer port name - just like in
regulator subsystem.
3. PHY driver receives an array of, let's say, phy_init_data inside its
platform data that it would use to register its PHYs.
4. Consumer drivers would have constant consumer port names and wouldn't
receive any information about PHYs from platform code.
Code example:
[Board file]
static const struct phy_consumer_data usb_20_phy0_consumers[] = {
{
.devname = foo-ehci,
.port = usbphy,
},
};
static const struct phy_consumer_data usb_20_phy1_consumers[] = {
{
.devname = foo-otg,
.port = otgphy,
},
};
static const struct phy_init_data my_phys[] = {
{
.name = USB 2.0 PHY 0,
.consumers = usb_20_phy0_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
},
{
.name = USB 2.0 PHY 1,
.consumers = usb_20_phy1_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
},
{ }
};
static const struct platform_device usb_phy_pdev = {
.name = foo-usbphy,
.id = -1,
.dev = {
.platform_data = my_phys,
},
};
[PHY driver]
static int foo_usbphy_probe(pdev)
{
struct foo_usbphy *foo;
struct phy_init_data *init_data = pdev-dev.platform_data;
/* ... */
// for each PHY in init_data {
phy_register(foo-phy[i], init_data[i]);
// }
/* ... */
}
[EHCI driver]
static int foo_ehci_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, usbphy);
/* ... */
}
[OTG driver]
static int foo_otg_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, otgphy);
/* ... */
}
Having to write platform data for everything gets old fast and the
code
duplication is pretty tedious...
Adding a single pointer is tedious? Where is the name that you
are
going to lookup going to come from? That code doesn't write
itself...
It's adding platform data in the first place that gets tedious - and
of
course there's also DT and ACPI to worry about, it's not just a case
of
platform data and then you're done. Pushing the lookup into library
code means that drivers don't have to worry about any of this stuff.
I agree, so just pass around the pointer to the phy and all is good. No
need to worry about DT or ACPI or anything else.
With Device Tree we don't have board files anymore. How would
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 15:36:00 Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
IMHO it would be better if you provided some code example, but let's
try to check if I understood you correctly.
8---
-
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
This should be controller_pdev, not phy_pdev, yes?
Right. A copy-pasto.
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
Or even just phy_get(pdev-dev), because phy_get() could be smart
enough to to set phy = dev-platform_data.
Unless you need more than one PHY in this driver...
--
--8
Is this what you mean?
That's what I was going to suggest too. The struct phy is defined in
the board file, which already knows about all the PHYs that exist in
the system. (Or perhaps it is allocated dynamically, so that when many
board files are present in the same kernel, only the entries listed in
the board file for the current system get created.)
Well, such dynamic allocation is a must. We don't accept non-multiplatform
aware code anymore, not even saying about multiboard.
Then the
structure's address is stored in the platform data and made available
to both the provider and the consumer.
Yes, technically this can work. You would still have to perform some kind
of synchronization to make sure that the PHY bound to this structure is
actually present. This is again technically doable (e.g. a list of
registered struct phys inside PHY core).
Even though the struct phy is defined (or allocated) in the board file,
its contents don't get filled in until the PHY driver provides the
details.
You can't assure this. Board file is free to do whatever it wants with
this struct. A clean solution would prevent this.
It's technically correct, but quality of this solution isn't really
nice, because it's a layering violation (at least if I understood
what you mean). This is because you need to have full definition of
struct phy in board file and a structure that is used as private data
in PHY core comes from platform code.
You don't have to have a full definition in the board file. Just a
partial definition -- most of the contents can be filled in later, when
the PHY driver is ready to store the private data.
It's not a layering violation for one region of the kernel to store
private data in a structure defined by another part of the kernel.
This happens all the time (e.g., dev_set_drvdata).
Not really. The phy struct is something that _is_ private data of PHY
subsystem, not something that can store private data of PHY subsystem
(sure it can store private data of particular PHY driver, but that's
another story) and only PHY subsystem should have access to its contents.
By the way, we need to consider other cases here as well, for example it
would be nice to have a single phy_get() function that works for both non-
DT and DT cases to make the consumer driver not have to worry whether it's
being probed from DT or not.
I'd suggest simply reusing the lookup method of regulator framework, just
as I suggested here:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813/focus=101661
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 11:04:14 Greg KH wrote:
On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself?
If
you
had a priv pointer to search from, then you could have just
passed
the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but
let's
try to check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are
really helpful.
8---
-
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
--
-
-8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data
structure unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would
change include/linux/platform_data/ata-pxa.h to have a phy pointer
in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call
to
phy_create(). Then you can use that pointer wherever that plaform
data
is available (i.e. whereever platform_data is at).
Hmm? So, do you suggest to call phy_create() from board file? What
phy_ops struct and other hardware parameters would it take?
The issue is that a string name is not going to scale at all,
as it
requires hard-coded information that will change over time (as
the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in
some,
not so uncommon cases, but there is already a lot of existing
lookup
solutions over the kernel and so there is no point in introducing
another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this
one
is taken care of, to show how the others should be handled as well.
There was a reason for introducing lookup solutions. The reason was
that in board file there is no way to get a pointer to something that
is going to be created much later in time. We don't do time travel
;-).
Please just pass the real phy pointer around, that's what it
is
there
for. Your board binding logic/code should be able to handle
this,
as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't
really
nice, because it's a layering violation (at least if I understood
what
you mean). This is because you need to have full definition of
struct
phy in board file and a structure that is used as private data in
PHY
core comes from platform code.
No, just a pointer, you don't need the full structure until you
get to some .c code that actually manipulates the phy itself, for
all other places, you are just dealing with a pointer and a
structure you never reference.
Does that make more sense?
Well, to the point that I think I now understood your suggestion.
Unfortunately the suggestion alone isn't really something that can be
done, considering how driver core and generic frameworks work.
Ok, given that I seem to be totally confused as to exactly how the
board-specific frameworks work, I'll take your word for it.
Well, they are working in a way that keeps separation of layers, making
things clean. Platform code should not (well, there might exist some in
tree hacks, but this should not be propagated) used to exchange data
between drivers, but rather to specify board specific parameters for
generic drivers. If drivers need to cooperate, there must be a dedicated
interface for
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier
in
this thread for details about that.)
I got copied in very late so don't have most of the thread I'm afraid,
I did try looking at web archives but didn't see a clear problem
statement. In any case this is why the APIs doing lookups do the
lookups in the context of the requesting device - devices ask for
whatever name they use locally.
What do you mean by locally?
The problem with the api was that the phy core wanted a id and a name to
create a phy, and then later other code was doing a lookup based on
the name and id (mushed together), because it knew that this device
was the one it wanted.
Just like the clock api, which, for multiple devices, has proven to
cause problems. I don't want to see us accept an api that we know has
issues in it now, I'd rather us fix it up properly.
Subsystems should be able to create ids how ever they want to, and not
rely on the code calling them to specify the names of the devices that
way, otherwise the api is just too fragile.
I think, that if you create a device, then just carry around the pointer
to that device (in this case a phy) and pass it to whatever other code
needs it. No need to do lookups on known names or anything else,
just normal pointers, with no problems for multiple devices, busses, or
naming issues.
PHY object is not a device, it is something that a device driver creates
(one or more instances of) when it is being probed.
But you created a 'struct device' for it, so I think of it as a device
be it virtual or real :)
You don't have a clean way to export this PHY object to other driver,
other than keeping this PHY on a list inside PHY core with some
well-known ID (e.g. device name + consumer port name/index, like in
regulator core) and then to use this well-known ID inside consumer
driver as a lookup key passed to phy_get();
Actually I think for PHY case, exactly the same way as used for
regulators might be completely fine:
1. Each PHY would have some kind of platform, non-unique name, that is
just used to print some messages (like the platform/board name of a
regulator).
2. Each PHY would have an array of consumers. Consumer specifier would
consist of consumer device name and consumer port name - just like in
regulator subsystem.
3. PHY driver receives an array of, let's say, phy_init_data inside its
platform data that it would use to register its PHYs.
4. Consumer drivers would have constant consumer port names and wouldn't
receive any information about PHYs from platform code.
Code example:
[Board file]
static const struct phy_consumer_data usb_20_phy0_consumers[] = {
{
.devname = foo-ehci,
.port = usbphy,
},
};
static const struct phy_consumer_data usb_20_phy1_consumers[] = {
{
.devname = foo-otg,
.port = otgphy,
},
};
static const struct phy_init_data my_phys[] = {
{
.name = USB 2.0 PHY 0,
.consumers = usb_20_phy0_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
},
{
.name = USB 2.0 PHY 1,
.consumers = usb_20_phy1_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
},
{ }
};
static const struct platform_device usb_phy_pdev = {
.name = foo-usbphy,
.id = -1,
.dev = {
.platform_data = my_phys,
},
};
[PHY driver]
static int foo_usbphy_probe(pdev)
{
struct foo_usbphy *foo;
struct phy_init_data *init_data = pdev-dev.platform_data;
/* ... */
// for each PHY in init_data {
phy_register(foo-phy[i], init_data[i]);
// }
/* ... */
}
[EHCI driver]
static int foo_ehci_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, usbphy);
/* ... */
}
[OTG driver]
static int foo_otg_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, otgphy);
/* ... */
}
That's not so bad, as long as you let the phy core use whatever name it
wants for the device when it registers it with sysfs. Use the name you
are requesting as a tag or some such hint as to what the phy can be
looked up by.
Good luck handling duplicate tags :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote: That's what I was going to suggest too. The struct phy is defined in the board file, which already knows about all the PHYs that exist in the system. (Or perhaps it is allocated dynamically, so that when many board files are present in the same kernel, only the entries listed in the board file for the current system get created.) Well, such dynamic allocation is a must. We don't accept non-multiplatform aware code anymore, not even saying about multiboard. Then the structure's address is stored in the platform data and made available to both the provider and the consumer. Yes, technically this can work. You would still have to perform some kind of synchronization to make sure that the PHY bound to this structure is actually present. This is again technically doable (e.g. a list of registered struct phys inside PHY core). The synchronization takes place inside phy_get. If phy_create hasn't been called for this structure by the time phy_get runs, phy_get will return an error. Even though the struct phy is defined (or allocated) in the board file, its contents don't get filled in until the PHY driver provides the details. You can't assure this. Board file is free to do whatever it wants with this struct. A clean solution would prevent this. I'm not sure what you mean here. Of course I can't prevent a board file from messing up a data structure. I can't prevent it from causing memory access violations either; in fact, I can't prevent any bugs in other people's code. Besides, why do you say the board file is free to do whatever it wants with the struct phy? Currently the struct phy is created by the PHY provider and the PHY core, right? It's not even mentioned in the board file. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. You don't have to have a full definition in the board file. Just a partial definition -- most of the contents can be filled in later, when the PHY driver is ready to store the private data. It's not a layering violation for one region of the kernel to store private data in a structure defined by another part of the kernel. This happens all the time (e.g., dev_set_drvdata). Not really. The phy struct is something that _is_ private data of PHY subsystem, not something that can store private data of PHY subsystem (sure it can store private data of particular PHY driver, but that's another story) and only PHY subsystem should have access to its contents. If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. By the way, we need to consider other cases here as well, for example it would be nice to have a single phy_get() function that works for both non- DT and DT cases to make the consumer driver not have to worry whether it's being probed from DT or not. You ought to be able to adapt this scheme to work with DT. Maybe by having multiple phy_address arrays. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
On Tue, Jul 23, 2013 at 10:17 PM, Chris Lee update...@gmail.com wrote: Not all tuners support all fec's Nitpick: tuner doesn't have anything to do with FEC, it just provides IQ outputs to the demodulator. ;-) That said; Demods support all FEC's relevant to their delivery systems. It's just that some devices likely do support some additional states. - genpix devices support an odd 5/11 fec for digicipher, pretty sure no one else does. I think DCII FEC5/11 is standard, reading this URL http://rickcaylor.websitetoolbox.com/post/DCII-Valid-SRFECModulation-Combinations-5827500 Also, according to the BCM4201 datasheet: * DVB/DIRECTV/Digicipher II compliant FEC decoder 64 state viterbi decoder supports rates= 5/11, 1/2, 3/5, 2/3, 3/4. 4/5, 5/6, 6/7, 7/8 I would say, it is pretty much standard for DCII. Given that it is pretty much standard, I would say that for DCII; for the genpix all you need is a SYS_DCII and or a SYS_DSS addition to the genpix driver, rather than having a ton of delivery systems mixed with modulations as in your patch with DCII_QPSK, ... _OQPSK etc. Actually, those are a bit too superfluous. You shouldn't mix delivery systems and modulations. That was the whole reason why the delivery system flag was introduced to make things saner and proper for the frontend API. If I am not mistaken, the genpix hardware is a hardware wrapper around the BCM demodulator. So, it is quite likely that even if you don't set any FEC parameter, the device could still acquire lock as expected. I am not holding my breath on this. Maybe someone with a genpix device can prove me right or wrong. - stv0899 supports 1/2, 2/3, 3/4, 5/6, 6/7, 7/8 - stv0900 supports 1/2, 3/5, 2/3, 3/4, 4/5, 5/6, 8/9, 9/10 Ah Though these devices support additional modes, the STB0899 (I don't know whether you meant the STB0899 with stv0899, yet looking at the stb0899, since there doesn't seem to be other references) With the STB0899 driver, all you need to tune with it is Frequency, Symbol Rate and Delivery system With the STV090x driver all you need is Frequency and Symbol Rate. (It will auto detect delivery system) Not all tuners support the entire range of fec's. I think this is more the norm then the exception. I find it slightly hard to believe... ;-) If the userland application can poll the driver for a list of supported fec it allows them to have a list of valid tuning options for the user to choose from, vs listing everything and hoping it doesnt fail. When a driver is not accepting those parameters as inputs, why should the application/user burden himself with knowing parameters of no relevance to him ? As stated Id much rather have a list made up from system - modulation - fec. ie genpix SYS_TURBO - QPSK/8PSK SYS_TURBO.QPSK - 1/2, 2/3, 3/4, 5/6, 7/8 SYS_TURBO.8PSK - 2/3, 3/4, 5/6, 8/9 but that could get more complicated to implement pretty quickly Actually with all those redundant FEC bits gone away from relevance, things are a bit more saner. Regards, Manu -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mb86a20s and cx23885
Hi As was a compilation error, I used git bisect skip. From what I've come up with something that I think is not what I'm looking for. Is it advisable to do it again? and where you get an error trying to git bisect bad and see where it arrived and then git bisec good again. what I got was: ... alfredo@linux-puon:/usr/src/git/linux git stash Saved working directory and index state WIP on (no branch): a08d2c7 [media] pwc: Remove driver specific ioctls HEAD is now at a08d2c7 [media] pwc: Remove driver specific ioctls alfredo@linux-puon:/usr/src/git/linux git bisect bad Bisecting: 92 revisions left to test after this (roughly 7 steps) [38e3d7ce41cff58bacebb2bcecf7d386c60b954b] [media] cx23885: Ensure the MPEG encoder height is configured from the norm alfredo@linux-puon:/usr/src/git/linux ... alfredo@linux-puon:/usr/src/git/linux git stash Saved working directory and index state WIP on (no branch): 38e3d7c [media] cx23885: Ensure the MPEG encoder height is configured from the norm HEAD is now at 38e3d7c [media] cx23885: Ensure the MPEG encoder height is configured from the norm alfredo@linux-puon:/usr/src/git/linux git bisect bad Bisecting: 45 revisions left to test after this (roughly 6 steps) [f9e54512fd16379812bcff86d95d0a7d78028b20] [media] af9005-fe: convert set_fontend to use DVBv5 parameters alfredo@linux-puon:/usr/src/git/linux ... alfredo@linux-puon:/usr/src/git/linux git stash Saved working directory and index state WIP on (no branch): f9e5451 [media] af9005-fe: convert set_fontend to use DVBv5 parameters HEAD is now at f9e5451 [media] af9005-fe: convert set_fontend to use DVBv5 parameters alfredo@linux-puon:/usr/src/git/linux git bisect good Bisecting: 22 revisions left to test after this (roughly 5 steps) [8de8594a79ae43b08d115c94f09373f6c673f202] [media] dvb-core: be sure that drivers won't use DVBv3 internally alfredo@linux-puon:/usr/src/git/linux ... alfredo@linux-puon:/usr/src/git/linux make CHK include/linux/version.h CHK include/generated/utsrelease.h CALLscripts/checksyscalls.sh CHK include/generated/compile.h CHK kernel/config_data.h CC fs/compat_ioctl.o fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: array type has incomplete element type fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: array type has incomplete element type fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: array type has incomplete element type fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1345:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: array type has incomplete element type fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: array type has incomplete element type fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: array type has incomplete element type fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1346:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_parameters’ fs/compat_ioctl.c:1347:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_event’ fs/compat_ioctl.c:1347:1: error: array type has incomplete element type fs/compat_ioctl.c:1347:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 16:53:55 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: That's what I was going to suggest too. The struct phy is defined in the board file, which already knows about all the PHYs that exist in the system. (Or perhaps it is allocated dynamically, so that when many board files are present in the same kernel, only the entries listed in the board file for the current system get created.) Well, such dynamic allocation is a must. We don't accept non-multiplatform aware code anymore, not even saying about multiboard. Then the structure's address is stored in the platform data and made available to both the provider and the consumer. Yes, technically this can work. You would still have to perform some kind of synchronization to make sure that the PHY bound to this structure is actually present. This is again technically doable (e.g. a list of registered struct phys inside PHY core). The synchronization takes place inside phy_get. If phy_create hasn't been called for this structure by the time phy_get runs, phy_get will return an error. Yes, this is the solution that I had in mind when saying that this is doable. Even though the struct phy is defined (or allocated) in the board file, its contents don't get filled in until the PHY driver provides the details. You can't assure this. Board file is free to do whatever it wants with this struct. A clean solution would prevent this. I'm not sure what you mean here. Of course I can't prevent a board file from messing up a data structure. I can't prevent it from causing memory access violations either; in fact, I can't prevent any bugs in other people's code. Besides, why do you say the board file is free to do whatever it wants with the struct phy? Currently the struct phy is created by the PHY provider and the PHY core, right? It's not even mentioned in the board file. I mean, if you have a struct type of which full declaration is available for some code, this code can access any memeber of it without any hacks, which is not something that we want to have in board files. The phy struct should be opaque for them. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. You don't have to have a full definition in the board file. Just a partial definition -- most of the contents can be filled in later, when the PHY driver is ready to store the private data. It's not a layering violation for one region of the kernel to store private data in a structure defined by another part of the kernel. This happens all the time (e.g., dev_set_drvdata). Not really. The phy struct is something that _is_ private data of PHY subsystem, not something that can store private data of PHY subsystem (sure it can store private data of particular PHY driver, but that's another story) and only PHY subsystem should have access to its contents. If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. OK, this can work. Again, just technically, because it's rather ugly. By the way, we need to consider other cases here as well, for example it would be nice to have a single phy_get() function that works for both non- DT and DT cases to make the consumer driver not have to worry whether it's being probed from DT or not. You ought to be able to adapt this scheme to work with DT. Maybe by having multiple phy_address arrays. Where would you want to have those phy_address arrays stored? There are no board files when booting with DT. Not even saying that you don't need to use any hacky schemes like this when you have DT that nicely specifies relations between devices. Anyway, board file should not be considered as a method to exchange data between drivers. It should be used only to pass data from it to drivers, not the other way. Ideally all data in a board file should be marked as const and __init and dropped after system initialization. Best regards, Tomasz -- To
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 13:50:07 Greg KH wrote:
On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back
earlier
in
this thread for details about that.)
I got copied in very late so don't have most of the thread I'm
afraid,
I did try looking at web archives but didn't see a clear problem
statement. In any case this is why the APIs doing lookups do the
lookups in the context of the requesting device - devices ask for
whatever name they use locally.
What do you mean by locally?
The problem with the api was that the phy core wanted a id and a
name to create a phy, and then later other code was doing a
lookup based on the name and id (mushed together), because it
knew that this device was the one it wanted.
Just like the clock api, which, for multiple devices, has proven to
cause problems. I don't want to see us accept an api that we know
has
issues in it now, I'd rather us fix it up properly.
Subsystems should be able to create ids how ever they want to, and
not
rely on the code calling them to specify the names of the devices
that
way, otherwise the api is just too fragile.
I think, that if you create a device, then just carry around the
pointer to that device (in this case a phy) and pass it to whatever
other code needs it. No need to do lookups on known names or
anything else, just normal pointers, with no problems for multiple
devices, busses, or naming issues.
PHY object is not a device, it is something that a device driver
creates (one or more instances of) when it is being probed.
But you created a 'struct device' for it, so I think of it as a device
be it virtual or real :)
Keep in mind that those virtual devices are created by PHY driver bound to
a real device and one real device can have multiple virtual devices behind
it.
You don't have a clean way to export this PHY object to other driver,
other than keeping this PHY on a list inside PHY core with some
well-known ID (e.g. device name + consumer port name/index, like in
regulator core) and then to use this well-known ID inside consumer
driver as a lookup key passed to phy_get();
Actually I think for PHY case, exactly the same way as used for
regulators might be completely fine:
1. Each PHY would have some kind of platform, non-unique name, that is
just used to print some messages (like the platform/board name of a
regulator).
2. Each PHY would have an array of consumers. Consumer specifier would
consist of consumer device name and consumer port name - just like in
regulator subsystem.
3. PHY driver receives an array of, let's say, phy_init_data inside
its
platform data that it would use to register its PHYs.
4. Consumer drivers would have constant consumer port names and
wouldn't receive any information about PHYs from platform code.
Code example:
[Board file]
static const struct phy_consumer_data usb_20_phy0_consumers[] = {
{
.devname = foo-ehci,
.port = usbphy,
},
};
static const struct phy_consumer_data usb_20_phy1_consumers[] = {
{
.devname = foo-otg,
.port = otgphy,
},
};
static const struct phy_init_data my_phys[] = {
{
.name = USB 2.0 PHY 0,
.consumers = usb_20_phy0_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
},
{
.name = USB 2.0 PHY 1,
.consumers = usb_20_phy1_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
},
{ }
};
static const struct platform_device usb_phy_pdev = {
.name = foo-usbphy,
.id = -1,
.dev = {
.platform_data = my_phys,
},
};
[PHY driver]
static int foo_usbphy_probe(pdev)
{
struct foo_usbphy *foo;
struct phy_init_data *init_data = pdev-dev.platform_data;
/* ... */
// for each PHY in init_data {
phy_register(foo-phy[i], init_data[i]);
// }
/* ... */
}
[EHCI driver]
static int foo_ehci_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, usbphy);
/* ... */
}
[OTG driver]
static int foo_otg_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, otgphy);
/* ... */
}
That's not so bad, as long as you let the phy core use whatever name it
wants for the device when it registers it with sysfs.
Yes, in regulator core
Re: Prof DVB-S2 USB device
On 23-07-13 18:52, Krishna Kishore wrote: #Sorry for sending to individual email ids Hi, I am trying to use Prof DVB-S2 USB device with Linux host. Device gets detected. But, I am facing the following problems. You will need to provide much more information then that. What does dmesg say? lsusb? what driver are you using, what kernel version? Are you using it as a module? Have you enabled debugging in your kernel? Those questions come to my mind. 1. It takes approximately 21 minutes to get /dev/dvb/adapter0/frontend0 and /dev/dvb/adapter0/demux0 to get created. This happens every time 2. After /dev/dvb/adapter0/frontend0 gets created, when I use w_scan utility to scan for channels, it does not list the channels. a. In dmesg logs, I see DEMOD LOCK FAIL error continuously. Paste your logs (or if its too much, only copy/paste the relevant parts. You ask for a limb, yet offer nothing. oliver Can you please help me? Regards, Kishore. SASKEN BUSINESS DISCLAIMER: This message may contain confidential, proprietary or legally privileged information. In case you are not the original intended Recipient of the message, you must not, directly or indirectly, use, disclose, distribute, print, or copy any part of this message and you are requested to delete it and inform the sender. Any views expressed in this message are those of the individual sender unless otherwise stated. Nothing contained in this message shall be construed as an offer or acceptance of any offer by Sasken Communication Technologies Limited (Sasken) unless sent with that express intent and with due authority of Sasken. Sasken has taken enough precautions to prevent the spread of viruses. However the company accepts no liability for any damage caused by any virus transmitted by this email. Read Disclaimer at http://www.sasken.com/extras/mail_disclaimer.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote: If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. OK, this can work. Again, just technically, because it's rather ugly. There's no reason the phy_address things have to be arrays. A separate individual pointer for each PHY would work just as well. Where would you want to have those phy_address arrays stored? There are no board files when booting with DT. Not even saying that you don't need to use any hacky schemes like this when you have DT that nicely specifies relations between devices. If everybody agrees DT has a nice scheme for specifying relations between devices, why not use that same scheme in the PHY core? Anyway, board file should not be considered as a method to exchange data between drivers. It should be used only to pass data from it to drivers, not the other way. Ideally all data in a board file should be marked as const and __init and dropped after system initialization. The phy_address things don't have to be defined or allocated in the board file; they could be set up along with the platform data. In any case, this was simply meant to be a suggestion to show that it is relatively easy to do what you need without using name or ID strings. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 11:05:48PM +0200, Tomasz Figa wrote: That's not so bad, as long as you let the phy core use whatever name it wants for the device when it registers it with sysfs. Yes, in regulator core consumer names are completely separated from this. Regulator core simply assigns a sequential integer ID to each regulator and registers /sys/class/regulator/regulator.ID for each regulator. Yes, that's fine. Use the name you are requesting as a tag or some such hint as to what the phy can be looked up by. Good luck handling duplicate tags :) The tag alone is not a key. Lookup key consists of two components, consumer device name and consumer tag. What kind of duplicate tags can be a problem here? Ok, I didn't realize it looked at both parts, that makes sense, thanks. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
Nitpick: tuner doesn't have anything to do with FEC, it just provides IQ outputs to the demodulator. ;-) ya ya :) you knew what I meant, not what I said hehe Demods support all FEC's relevant to their delivery systems. It's just that some devices likely do support some additional states. This part I dont understand, what do you mean additional states ? and how would a userland application determine if a demod supports these additional states? I think DCII FEC5/11 is standard, reading this URL http://rickcaylor.websitetoolbox.com/post/DCII-Valid-SRFECModulation-Combinations-5827500 I would say, it is pretty much standard for DCII. yes 5/11 is standard for DCII, but nothing else. Given that it is pretty much standard, I would say that for DCII; for the genpix all you need is a SYS_DCII and or a SYS_DSS addition to the genpix driver, rather than having a ton of delivery systems mixed with modulations as in your patch with DCII_QPSK, ... _OQPSK etc. Actually, those are a bit too superfluous. You shouldn't mix delivery systems and modulations. That was the whole reason why the delivery system flag was introduced to make things saner and proper for the frontend API. Yup fair enough, easy to change, I'll get on that and resubmit the patch. If I am not mistaken, the genpix hardware is a hardware wrapper around the BCM demodulator. So, it is quite likely that even if you don't set any FEC parameter, the device could still acquire lock as expected. I am not holding my breath on this. Maybe someone with a genpix device can prove me right or wrong. FEC_AUTO works for all but turbo-qpsk on genpix devices. I still think its important to have all the fec supported in the driver though even if FEC_AUTO did work 100% else why even have it as an option at all. With the STB0899 driver, all you need to tune with it is Frequency, Symbol Rate and Delivery system With the STV090x driver all you need is Frequency and Symbol Rate. (It will auto detect delivery system) Same thing, I still think if we allow the user to send a fec value we should make sure its right, else why not just hard code all the drivers to fec-auto that support it and remove the option all together. I dont like that option. When a driver is not accepting those parameters as inputs, why should the application/user burden himself with knowing parameters of no relevance to him ? But it will accept them as inputs. without complaint too. I can send DTV_INNER_FEC w/ FEC_5_11 to stv090x and it doesnt complain at all, even though it doesnt support it. It'll even acquire a lock just because the demod uses blind search. So the driver most definitely does accept fec that it cant use. Actually with all those redundant FEC bits gone away from relevance, things are a bit more saner. I dont understand this either. gone away from relevance are you meaning just how they really arent used much anymore or something? still though if the demod supports them I think we should too. Honestly I still think the .delsys .delmod .delfec is a cleaner approach then we have now which is ugly and mismatched (modulations mixed in with fec, and only some are defined) its not a perfect solution though so I really dont think its worth fighting for if others dont agree with me. Im just kinda surprised that everyone is perfectly happy with the .delsys / .caps method we use Chris -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. OK, this can work. Again, just technically, because it's rather ugly. There's no reason the phy_address things have to be arrays. A separate individual pointer for each PHY would work just as well. Where would you want to have those phy_address arrays stored? There are no board files when booting with DT. Not even saying that you don't need to use any hacky schemes like this when you have DT that nicely specifies relations between devices. If everybody agrees DT has a nice scheme for specifying relations between devices, why not use that same scheme in the PHY core? It is already used, for cases when consumer device has a DT node attached. In non-DT case this kind lookup translates loosely to something that is being done in regulator framework - you can't bind devices by pointers, because you don't have those pointers, so you need to use device names. Anyway, board file should not be considered as a method to exchange data between drivers. It should be used only to pass data from it to drivers, not the other way. Ideally all data in a board file should be marked as const and __init and dropped after system initialization. The phy_address things don't have to be defined or allocated in the board file; they could be set up along with the platform data. There is no platform data when booting with DT. In any case, this was simply meant to be a suggestion to show that it is relatively easy to do what you need without using name or ID strings. Sure. It's good to have different options discussed as well. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mb86a20s and cx23885
Hi I forgot, in this section I put BAD because not have picture or sound, but if signal. alfredo@linux-puon:/usr/src/git/linux git stash Saved working directory and index state WIP on (no branch): 2827e1f [media] tlg2300: convert set_fontend to use DVBv5 parameters HEAD is now at 2827e1f [media] tlg2300: convert set_fontend to use DVBv5 parameters alfredo@linux-puon:/usr/src/git/linux git bisect bad /*apear tunner, but not tunner*/ Bisecting: 4 revisions left to test after this (roughly 3 steps) [4fa102d5cc5b412fa3bc7cc8c24e4d9052e4f693] [media] vp702x-fe: convert set_fontend to use DVBv5 parameters Is there a way to return to after with bisect without compile all? Thanks, Alfredo -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: width and height of JPEG compressed images
Hi Sylwester, On Sun, Jul 21, 2013 at 10:38:18PM +0200, Sylwester Nawrocki wrote: On 07/19/2013 10:28 PM, Sakari Ailus wrote: On Sat, Jul 06, 2013 at 09:58:23PM +0200, Sylwester Nawrocki wrote: On 07/05/2013 10:22 AM, Thomas Vajzovic wrote: Hello, I am writing a driver for the sensor MT9D131. This device supports digital zoom and JPEG compression. Although I am writing it for my company's internal purposes, it will be made open-source, so I would like to keep the API as portable as possible. The hardware reads AxB sensor pixels from its array, resamples them to CxD image pixels, and then compresses them to ExF bytes. The subdevice driver sets size AxB to the value it receives from v4l2_subdev_video_ops.s_crop(). To enable compression then v4l2_subdev_video_ops.s_mbus_fmt() is called with fmt-code=V4L2_MBUS_FMT_JPEG_1X8. fmt-width and fmt-height then ought to specify the size of the compressed image ExF, that is, the size specified is the size in the format specified (the number of JPEG_1X8), not the size it would be in a raw format. In VIDIOC_S_FMT 'sizeimage' specifies size of the buffer for the compressed frame at the bridge driver side. And width/height should specify size of the re-sampled (binning, skipping ?) frame - CxD, if I understand what you are saying correctly. I don't quite what transformation is done at CxD - ExF. Why you are using ExF (two numbers) to specify number of bytes ? And how can you know exactly beforehand what is the frame size after compression ? Does the sensor transmit fixed number of bytes per frame, by adding some padding bytes if required to the compressed frame data ? Is it something like: sensor matrix (AxB pixels) - binning/skipping (CxD pixels) - - JPEG compresion (width = C, height = D, sizeimage ExF bytes) ? This allows the bridge driver to be compression agnostic. It gets told how many bytes to allocate per buffer and it reads that many bytes. It doesn't have to understand that the number of bytes isn't directly related to the number of pixels. So how does the user tell the driver what size image to capture before compression, CxD? I think you should use VIDIOC_S_FMT(width = C, height = D, sizeimage = ExF) for that. And s_frame_desc sudev op could be used to pass sizeimage to the sensor subdev driver. Agreed. Let me take this into account in the next RFC. Thanks. (or alternatively, if you disagree and think CxD should be specified by s_fmt(), then how does the user specify ExF?) Does the user need to specify ExF, for other purposes than limiting the size of the image? I would leave this up to the sensor driver (with reasonable alignment). The sensor driver would tell about this to the receiver through AFAIU ExF is closely related to the memory buffer size, so the sensor driver itself wouldn't have enough information to fix up ExF, would it ? If the desired sizeimage is known, F can be calculated if E is fixed, say 1024 should probably work for everyone, shoulnd't it? frame descriptors. (But still I don't think frame descriptors should be settable; what sensors can support is fully sensor specific and the parameters that typically need to be changed are quite limited in numbers. So I'd go with e.g. controls, again.) I agree it would have been much more clear to have read only frame descriptors outside of the subdev. But the issue with controls is that it would have been difficult to define same parameter for multiple logical stream on the data bus. And data interleaving is a standard feature, it is well defined in the MIPI CSI-2 specification. So my feeling is that we would be better off with data structure and a callback, rather than creating multiple strange controls. That's true for controls. I'd hope that we could connect properties (or extended^2 controls or whatever) to arbitrary attributes vs. subdevs or V4L2 devices currently. But we'd need the the properties first in that case. In the meantime I'd be fine with even a few funnily named controls. However if we don't use media bus format callbacks, nor frame descriptor callbacks, then what ?... :) It sounds reasonable to me to have frame frame descriptor defined by the sensor (data source) based on media bus format, frame interval, link frequency, etc. Problematic seem to be parameters that are now handled on the video node side, like, e.g. buffer size. Is that really problematic? If my memory serves me right, passing the image size in frame descriptor was done for the reason that controls are associated with a device rather than a more fine grained object and a bridge driver didn't have a good way to tell the sensor driver its control x was since unchangeable. Resolving the two would make it possible to meaningfully use controls for the purpose as far as I understand. (The second is easy: make v4l2_ctrl_grab() use integer rather than a single bit. Possibly ensure the coupl:e of users are
[PATCH] gp8psk: add DSS/DCII tuning, fix turbofec fec values, add returning actual tuned values after lock
Revised patch, I seperated the DCII systems into their correct DCII system and
4x different modulations.
Chris Lee
---
drivers/media/usb/dvb-usb/gp8psk-fe.c | 263 +-
drivers/media/usb/dvb-usb/gp8psk.h| 2 +
include/uapi/linux/dvb/frontend.h | 6 +
3 files changed, 201 insertions(+), 70 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/gp8psk-fe.c
b/drivers/media/usb/dvb-usb/gp8psk-fe.c
index 67957dd..77ba995 100644
--- a/drivers/media/usb/dvb-usb/gp8psk-fe.c
+++ b/drivers/media/usb/dvb-usb/gp8psk-fe.c
@@ -53,7 +53,43 @@ static int gp8psk_fe_update_status(struct gp8psk_fe_state
*st)
static int gp8psk_fe_read_status(struct dvb_frontend* fe, fe_status_t *status)
{
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
struct gp8psk_fe_state *st = fe-demodulator_priv;
+
+ u8 buf[32];
+ int frequency;
+ int carrier_error;
+ int carrier_offset;
+ int rate_error;
+ int rate_offset;
+ int symbol_rate;
+
+ int fe_gp8psk_system_return[] = {
+ SYS_DVBS,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_TURBO,
+ SYS_DCII,
+ SYS_DCII,
+ SYS_DCII,
+ SYS_DCII,
+ SYS_DSS,
+ SYS_UNDEFINED
+ };
+
+ int fe_gp8psk_modulation_return[] = {
+ QPSK,
+ QPSK,
+ PSK_8,
+ QAM_16,
+ C_QPSK,
+ I_QPSK,
+ Q_QPSK,
+ C_OQPSK,
+ QPSK,
+ QPSK,
+ };
+
gp8psk_fe_update_status(st);
if (st-lock)
@@ -61,10 +97,79 @@ static int gp8psk_fe_read_status(struct dvb_frontend* fe,
fe_status_t *status)
else
*status = 0;
- if (*status FE_HAS_LOCK)
+ if (*status FE_HAS_LOCK) {
+ gp8psk_usb_in_op(st-d, GET_SIGNAL_STAT, 0, 0, buf, 32);
+ frequency = ((buf[11] 24) + (buf[10] 16) +
(buf[9] 8) + buf[8]) / 1000;
+ carrier_error = ((buf[15] 24) + (buf[14] 16) + (buf[13]
8) + buf[12]) / 1000;
+ carrier_offset = (buf[19] 24) + (buf[18] 16) + (buf[17]
8) + buf[16];
+ rate_error = (buf[23] 24) + (buf[22] 16) +
(buf[21] 8) + buf[20];
+ rate_offset = (buf[27] 24) + (buf[26] 16) +
(buf[25] 8) + buf[24];
+ symbol_rate = (buf[31] 24) + (buf[30] 16) +
(buf[29] 8) + buf[28];
+
+ c-frequency= frequency - carrier_error;
+ c-symbol_rate = symbol_rate + rate_error;
+
+ switch (c-delivery_system) {
+ case SYS_DSS:
+ case SYS_DVBS:
+ c-delivery_system =
fe_gp8psk_system_return[buf[1]];
+ c-modulation =
fe_gp8psk_modulation_return[buf[1]];
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_1_2; break;
+ case 1: c-fec_inner = FEC_2_3; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_5_6; break;
+ case 4: c-fec_inner = FEC_6_7; break;
+ case 5: c-fec_inner = FEC_7_8; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ break;
+ case SYS_TURBO:
+ c-delivery_system =
fe_gp8psk_system_return[buf[1]];
+ c-modulation =
fe_gp8psk_modulation_return[buf[1]];
+ if (c-modulation == QPSK) {
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_7_8; break;
+ case 1: c-fec_inner = FEC_1_2; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_2_3; break;
+ case 4: c-fec_inner = FEC_5_6; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ } else {
+ switch (buf[2]) {
+ case 0: c-fec_inner = FEC_2_3; break;
+ case 1: c-fec_inner = FEC_3_4; break;
+ case 2: c-fec_inner = FEC_3_4; break;
+ case 3: c-fec_inner = FEC_5_6; break;
+ case 4: c-fec_inner = FEC_8_9; break;
+ default: c-fec_inner = FEC_AUTO; break;
+ }
+ }
+ break;
+ case SYS_DCII:
+ c-modulation
[PATCH] S2255: Removal of unnecessary videobuf_queue_is_busy
Removes unnecessary query of buffer state. The code already checks if stream
is active or not.
Signed-off-by: Dean Anderson linux-...@sensoray.com
---
drivers/media/usb/s2255/s2255drv.c |9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index ab97e7d..6bc9b8e 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -1,7 +1,7 @@
/*
* s2255drv.c - a driver for the Sensoray 2255 USB video capture device
*
- * Copyright (C) 2007-2010 by Sensoray Company Inc.
+ * Copyright (C) 2007-2013 by Sensoray Company Inc.
* Dean Anderson
*
* Some video buffer code based on vivi driver:
@@ -52,7 +52,7 @@
#include media/v4l2-ctrls.h
#include media/v4l2-event.h
-#define S2255_VERSION 1.22.1
+#define S2255_VERSION 1.23.1
#define FIRMWARE_FILE_NAME f2255usb.bin
/* default JPEG quality */
@@ -1303,11 +1303,6 @@ static int vidioc_s_std(struct file *file, void *priv,
v4l2_std_id i)
int ret = 0;
mutex_lock(q-vb_lock);
- if (videobuf_queue_is_busy(q)) {
- dprintk(1, queue busy\n);
- ret = -EBUSY;
- goto out_s_std;
- }
if (res_locked(fh)) {
dprintk(1, can't change standard after started\n);
ret = -EBUSY;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
On Wed, Jul 24, 2013 at 2:57 AM, Chris Lee update...@gmail.com wrote: Nitpick: tuner doesn't have anything to do with FEC, it just provides IQ outputs to the demodulator. ;-) ya ya :) you knew what I meant, not what I said hehe Demods support all FEC's relevant to their delivery systems. It's just that some devices likely do support some additional states. This part I dont understand, what do you mean additional states ? and how would a userland application determine if a demod supports these additional states? Actually, the userland application shouldn't know about these. If I am not mistaken, the genpix hardware is a hardware wrapper around the BCM demodulator. So, it is quite likely that even if you don't set any FEC parameter, the device could still acquire lock as expected. I am not holding my breath on this. Maybe someone with a genpix device can prove me right or wrong. FEC_AUTO works for all but turbo-qpsk on genpix devices. That was why the SYS_TURBO flag was introduced. IIRC, you needed one flag alone for the turbo mode. I still think its important to have all the fec supported in the driver though even if FEC_AUTO did work 100% else why even have it as an option at all. Maybe, FEC_AUTO is broken for some very old hardware. If FEC_AUTO works just as expected, why would you have to take the gigantic effort of specifying parameters by hand which is error prone which you have mentioned later on ? I fail to understand your point. With the STB0899 driver, all you need to tune with it is Frequency, Symbol Rate and Delivery system With the STV090x driver all you need is Frequency and Symbol Rate. (It will auto detect delivery system) Same thing, I still think if we allow the user to send a fec value we should make sure its right, else why not just hard code all the drivers to fec-auto that support it and remove the option all together. I dont like that option. This is why it was decided eventually that the FEC bits are redundant and we decided not to create large lists and enumerations causing insanity and not to mention ugliness. AFAIR, almost all drivers do FEC_AUTO, except for the ones which have some known issues. When a driver is not accepting those parameters as inputs, why should the application/user burden himself with knowing parameters of no relevance to him ? But it will accept them as inputs. without complaint too. I can send DTV_INNER_FEC w/ FEC_5_11 to stv090x and it doesnt complain at all, even though it doesnt support it. It'll even acquire a lock just because the demod uses blind search. So the driver most definitely does accept fec that it cant use. The driver will acquire a lock to the frequency/srate and return the relevant FEC value for the user/application. This avoids pitfalls and human errors in manually specifying FEC bits to tune configurations, as I described above. Because some legacy application does set a FEC value which might be wrong and the rest are correct, I wouldn't fail that request. Actually with all those redundant FEC bits gone away from relevance, things are a bit more saner. I dont understand this either. gone away from relevance are you meaning just how they really arent used much anymore or something? still though if the demod supports them I think we should too. Yeah, they aren't really used at all. They exist for compatibility reasons. Manu -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
The problems isnt for tuners where FEC_AUTO does work, its more for ones that dont work like the genpix. Im sure there are others too. I still think that userland applications should be able to poll that info and that the ability to poll the info is a good thing not a bad thing. oh well, lets let this patch die, and the idea can be revisited in the future if it warrants more of a pressing need. Chris -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
On Tue, Jul 23, 2013 at 3:57 PM, Chris Lee update...@gmail.com wrote: The problems isnt for tuners where FEC_AUTO does work, its more for ones that dont work like the genpix. Im sure there are others too. If FEC_AUTO for turbo qpsk can be fixed in the Genpix firmware, maybe it's worth seeing if Genpix will have a look into it.? -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Proposed modifications to dvb_frontend_ops
He is, I talked to him last month about various things and he mentioned turbofec-qpsk FEC_AUTO is semi working and its in his plans. Chris On Tue, Jul 23, 2013 at 5:39 PM, VDR User user@gmail.com wrote: On Tue, Jul 23, 2013 at 3:57 PM, Chris Lee update...@gmail.com wrote: The problems isnt for tuners where FEC_AUTO does work, its more for ones that dont work like the genpix. Im sure there are others too. If FEC_AUTO for turbo qpsk can be fixed in the Genpix firmware, maybe it's worth seeing if Genpix will have a look into it.? -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 12:44:23PM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote: statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. What do you mean by locally? Within themselves - for example a regulator consumer asks for a given supply on the device in terms of the supply names the device has. The problem with the api was that the phy core wanted a id and a name to create a phy, and then later other code was doing a lookup based on the name and id (mushed together), because it knew that this device was the one it wanted. Ah, that sounds like the API is missing a component to link things together. But I could be wrong. What I would expect to see is that the consumer says I want the PHY called X and the PHY driver says I provide this set of PHYs with a layer in between that plugs those together. This would normally involve talking about the parent device rather than the PHY itself. I think, that if you create a device, then just carry around the pointer to that device (in this case a phy) and pass it to whatever other code needs it. No need to do lookups on known names or anything else, just normal pointers, with no problems for multiple devices, busses, or naming issues. I think you're not really talking about the lookup API at all here but rather about one way in which the matching code can be written. What everything *really* wants to do is work in terms of resources namespaced within struct devices since every bit of hardware in the system should have one of those it can use and if you have a struct device you can do useful things like call dev_printk() and find the device tree data to do device tree based lookups. Unfortunately for a number of buses even when statically registering the struct device doesn't get allocated until the device is probed so what everyone fell back on doing was using dev_name() in cases where the struct device wasn't there yet, or just always using it for consistency since for most of the affected buses dev_name() is fixed for human interface reasons. I think this is the issue you're concerned about here since if the dev_name() is dynamically allocated this breaks down. This only affects board files, DT and ACPI can both use their own data structures to do the mapping. I had thought you were talking about picking the names that the consumers use (which isn't actually that big a deal, it's just a bit annoying for the clock API). It's adding platform data in the first place that gets tedious - and of course there's also DT and ACPI to worry about, it's not just a case of platform data and then you're done. Pushing the lookup into library code means that drivers don't have to worry about any of this stuff. I agree, so just pass around the pointer to the phy and all is good. No need to worry about DT or ACPI or anything else. No, in practice passing around the pointer gets tricky if you're using something other than board files (or even are doing any kind of dynamic stuff with board files) since the two devices need to find each other and if you're using platform data then the code doing the matching has to know about the platform data for every device it might need to match which is just miserable. Something would need to do something like allocate the PHY objects and then arrange for them to be passed to both provider and consumer devices prior to those being registered, knowing where to place the pointers in the platform data for each device. This is straightforward with board files but not otherwise, people have tried this before. For most of the APIs doing this there is a clear and unambiguous name in the hardware that can be used (and for hardware process reasons is unlikely to get changed). The major exception to this is the clock API since it is relatively rare to have clear, segregated IP level information for IPs baked into larger chips. The other APIs tend to be establishing chip to chip links. The clock api is having problems with multiple names due to dynamic devices from what I was told. I want to prevent the PHY interface from having that same issue. I think the underlying issue here is that we don't have a good enough general way for board files (or other C code but mostly them) to talk about devices prior to their being registered - rather than have the pointer you're talking about be the PHY object itself have the pointer be something which allows us to match the struct device when it's created. This should be transparent to drivers and would be usable by all the existing APIs. signature.asc Description: Digital signature
Vážení E-mail užívateľa;
Vážení E-mail užívateľa; Prekročili ste 23432 boxy nastaviť svoje Webová služba / Administrátor, a budete mať problémy pri odosielaní a prijímať e-maily, kým znova overiť. Musíte aktualizovať kliknutím na odkaz nižšie a vyplňte údaje pre overenie vášho účtu Prosím, kliknite na odkaz nižšie alebo skopírovať vložiť do e-prehliadač pre overenie Schránky. http://webmailupdate29134.jimdo.com/ Pozor! Ak tak neurobíte, budú mať obmedzený prístup k e-mailu schránky. Ak sa nepodarí aktualizovať svoj účet do troch dní od aktualizácie oznámenia, bude váš účet natrvalo uzavretá. S pozdravom, System Administrator ® -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
