Re: width and height of JPEG compressed images
Hi Sakari, 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 ? 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. 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. -- Regards, Sylwester -- 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 Sun, 21 Jul 2013, Sylwester Nawrocki wrote: > > What's wrong with the platform_data structure, why can't that be used > > for this? > > At the point the platform data of some driver is initialized, e.g. in > board setup code the PHY pointer is not known, since the PHY supplier > driver has not initialized yet. Even though we wanted to pass pointer > to a PHY through some notifier call, it would have been not clear > which PHY user driver should match on such notifier. A single PHY > supplier driver can create M PHY objects and this needs to be mapped > to N PHY user drivers. This mapping needs to be defined somewhere by > the system integrator. It works well with device tree, but except that > there seems to be no other reliable infrastructure in the kernel to > define links/dependencies between devices, since device identifiers are > not known in advance in all cases. > > What Tomasz proposed seems currently most reasonable to me for non-dt. > > > Or, if not, we can always add pointers to the platform device structure, > > or even the main 'struct device' as well, that's what it is there for. > > Still we would need to solve a problem which platform device structure > gets which PHY pointer. Can you explain the issues in more detail? I don't fully understand the situation. Here's what I think I know: 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? 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? 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. The PHY's driver (the supplier) uses the platform data to construct a platform_device structure that represents the PHY. Until this is done, the controller's driver (the client) cannot use the PHY. 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. 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). Probably some of the details above are wrong; please indicate where I have gone astray. Also, I'm not clear about the role played by various APIs. For example, where does phy_create() fit into this picture? 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
cron job: media_tree daily build: WARNINGS
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: Sun Jul 21 19:00:21 CEST 2013 git branch: test git hash: 1c26190a8d492adadac4711fe5762d46204b18b0 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: WARNINGS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: WARNINGS linux-2.6.34.7-i686: WARNINGS linux-2.6.35.9-i686: WARNINGS linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: OK linux-3.10-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: WARNINGS linux-3.9.2-i686: WARNINGS linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: WARNINGS linux-2.6.35.9-x86_64: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: OK linux-3.10-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9.2-x86_64: WARNINGS apps: WARNINGS spec-git: OK sparse version: v0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.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] crystalhd gst: Port to GStreamer 1.0 API
Hi,
On Mon, Apr 01, 2013 at 01:26:54PM +0200, Guido Günther wrote:
> Hi,
> On Mon, Mar 11, 2013 at 09:40:22AM +0100, Guido Günther wrote:
> > ---
> > Tested with totem as well as gst-launch. Any chance this can be applied
> > to http://git.linuxtv.org/jarod/crystalhd.git?
>
> Any chance this gets applied.
Ping? It would really be nice to have a gstreamer 1.0 version.
Cheers,
-- Guido
>
> > Cheers,
> > -- Guido
> >
> > filters/gst/gst-plugin/configure.ac|8 +-
> > filters/gst/gst-plugin/src/gstbcmdec.c | 431
> >
> > filters/gst/gst-plugin/src/gstbcmdec.h | 68 ++---
> > 3 files changed, 248 insertions(+), 259 deletions(-)
> >
> > diff --git a/filters/gst/gst-plugin/configure.ac
> > b/filters/gst/gst-plugin/configure.ac
> > index 99b3713..a3c6467 100644
> > --- a/filters/gst/gst-plugin/configure.ac
> > +++ b/filters/gst/gst-plugin/configure.ac
> > @@ -1,9 +1,9 @@
> > AC_INIT
> >
> > dnl versions of gstreamer and plugins-base
> > -GST_MAJORMINOR=0.10
> > -GST_REQUIRED=0.10.0
> > -GSTPB_REQUIRED=0.10.0
> > +GST_MAJORMINOR=1.0
> > +GST_REQUIRED=1.0
> > +GSTPB_REQUIRED=1.0
> >
> > dnl fill in your package name and version here
> > dnl the fourth (nano) number should be 0 for a release, 1 for CVS,
> > @@ -56,7 +56,7 @@ dnl And we can also ask for the right version of gstreamer
> >
> > PKG_CHECK_MODULES(GST, \
> >gstreamer-$GST_MAJORMINOR >= $GST_REQUIRED
> > - gstreamer-video-0.10,
> > + gstreamer-video-1.0,
> >HAVE_GST=yes,HAVE_GST=no)
> >
> > dnl Give error and exit if we don't have gstreamer
> > diff --git a/filters/gst/gst-plugin/src/gstbcmdec.c
> > b/filters/gst/gst-plugin/src/gstbcmdec.c
> > index ed01c14..a51bd59 100644
> > --- a/filters/gst/gst-plugin/src/gstbcmdec.c
> > +++ b/filters/gst/gst-plugin/src/gstbcmdec.c
> > @@ -8,6 +8,7 @@
> > * AU
> > *
> > * HISTORY:
> > + * Updated for 1.0 by Guido Guenther
> > *
> > ***
> > *
> > @@ -40,6 +41,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #ifdef HAVE_CONFIG_H
> > #include
> > @@ -51,8 +53,8 @@
> > #include "parse.h"
> > #include "gstbcmdec.h"
> >
> > -GST_DEBUG_CATEGORY_STATIC (gst_bcmdec_debug);
> > -#define GST_CAT_DEFAULT gst_bcmdec_debug
> > +GST_DEBUG_CATEGORY_STATIC (gst_bcm_dec_debug);
> > +#define GST_CAT_DEFAULT gst_bcm_dec_debug
> >
> > //#define YV12__
> >
> > @@ -64,16 +66,18 @@ static GstFlowReturn
> > bcmdec_send_buff_detect_error(GstBcmDec *bcmdec, GstBuffer
> >guint8 flags)
> > {
> > BC_STATUS sts = BC_STS_SUCCESS;
> > + GstMapInfo info;
> >
> > GST_DEBUG_OBJECT(bcmdec, "Attempting to Send Buffer");
> >
> > sts = decif_send_buffer(&bcmdec->decif, pbuffer, size, tCurrent, flags);
> >
> > if (sts != BC_STS_SUCCESS) {
> > + gst_buffer_map(buf, &info, GST_MAP_READ);
> > GST_ERROR_OBJECT(bcmdec, "proc input failed sts = %d", sts);
> > GST_ERROR_OBJECT(bcmdec, "Chain: timeStamp = %llu size = %d
> > data = %p",
> > -GST_BUFFER_TIMESTAMP(buf),
> > GST_BUFFER_SIZE(buf),
> > -GST_BUFFER_DATA (buf));
> > +GST_BUFFER_DTS(buf), info.size, info.data);
> > + gst_buffer_unmap(buf, &info);
> > return GST_FLOW_ERROR;
> > }
> >
> > @@ -92,7 +96,7 @@ enum {
> > };
> >
> >
> > -GLB_INST_STS *g_inst_sts = NULL;
> > +static GLB_INST_STS *g_inst_sts = NULL;
> >
> > /*
> > * the capabilities of the inputs and outputs.
> > @@ -111,35 +115,29 @@ GstStaticPadTemplate sink_factory_bcm70012 =
> > GST_STATIC_PAD_TEMPLATE("sink", GST
> >
> > #ifdef YV12__
> > static GstStaticPadTemplate src_factory = GST_STATIC_PAD_TEMPLATE("src",
> > GST_PAD_SRC, GST_PAD_ALWAYS,
> > - GST_STATIC_CAPS("video/x-raw-yuv, " "format = (fourcc) { YV12
> > }, " "width = (int) [ 1, MAX ], "
> > + GST_STATIC_CAPS("video/x-raw, " "format = (string) { YV12 }, "
> > "width = (int) [ 1, MAX ], "
> > "height = (int) [ 1, MAX ], " "framerate =
> > (fraction) [ 0/1, 2147483647/1 ]"));
> > #define BUF_MULT (12 / 8)
> > #define BUF_MODE MODE420
> > #else
> > static GstStaticPadTemplate src_factory = GST_STATIC_PAD_TEMPLATE("src",
> > GST_PAD_SRC, GST_PAD_ALWAYS,
> > - GST_STATIC_CAPS("video/x-raw-yuv, " "format = (fourcc) { YUY2 }
> > , " "framerate = (fraction) [0,MAX], "
> > - "width = (int) [1,MAX], " "height = (int)
> > [1,MAX]; " "video/x-raw-yuv, "
> > - "format = (fourcc) { UYVY } , " "framerate =
> > (fraction) [0,MAX], " "width = (int) [1,MAX], "
> > + GST_STATIC_CAPS("video/x-raw, " "format = (string) { YUY2 } , "
> > "framerate = (fraction) [0,MAX], "
> > + "width = (int) [1,MAX], " "height = (int)
> > [1
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On 07/21/2013 05:48 PM, Greg KH wrote: On Sun, Jul 21, 2013 at 12:22:48PM +0200, Sascha Hauer wrote: On Sat, Jul 20, 2013 at 07:59:10PM -0700, Greg KH wrote: On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote: On Sat, 20 Jul 2013, Greg KH wrote: That should be passed using platform data. Ick, don't pass strings around, pass pointers. If you have platform data you can get to, then put the pointer there, don't use a "name". I don't think I understood you here :-s We wont have phy pointer when we create the device for the controller no?(it'll be done in board file). Probably I'm missing something. Why will you not have that pointer? You can't rely on the "name" as the device id will not match up, so you should be able to rely on the pointer being in the structure that the board sets up, right? Don't use names, especially as ids can, and will, change, that is going to cause big problems. Use pointers, this is C, we are supposed to be doing that :) Kishon, I think what Greg means is this: The name you are using must be stored somewhere in a data structure constructed by the board file, right? Or at least, associated with some data structure somehow. Otherwise the platform code wouldn't know which PHY hardware corresponded to a particular name. Greg's suggestion is that you store the address of that data structure in the platform data instead of storing the name string. Have the consumer pass the data structure's address when it calls phy_create, instead of passing the name. Then you don't have to worry about two PHYs accidentally ending up with the same name or any other similar problems. Close, but the issue is that whatever returns from phy_create() should then be used, no need to call any "find" functions, as you can just use the pointer that phy_create() returns. Much like all other class api functions in the kernel work. I think the problem here is to connect two from the bus structure completely independent devices. Several frameworks (ASoC, soc-camera) had this problem and this wasn't solved until the advent of devicetrees and their phandles. phy_create might be called from the probe function of some i2c device (the phy device) and the resulting pointer is then needed in some other platform devices (the user of the phy) probe function. The best solution we have right now is implemented in the clk framework which uses a string matching of the device names in clk_get() (at least in the non-dt case). I would argue that clocks are wrong here as well, as others have already pointed out. What's wrong with the platform_data structure, why can't that be used for this? At the point the platform data of some driver is initialized, e.g. in board setup code the PHY pointer is not known, since the PHY supplier driver has not initialized yet. Even though we wanted to pass pointer to a PHY through some notifier call, it would have been not clear which PHY user driver should match on such notifier. A single PHY supplier driver can create M PHY objects and this needs to be mapped to N PHY user drivers. This mapping needs to be defined somewhere by the system integrator. It works well with device tree, but except that there seems to be no other reliable infrastructure in the kernel to define links/dependencies between devices, since device identifiers are not known in advance in all cases. What Tomasz proposed seems currently most reasonable to me for non-dt. Or, if not, we can always add pointers to the platform device structure, or even the main 'struct device' as well, that's what it is there for. Still we would need to solve a problem which platform device structure gets which PHY pointer. -- Regards, Sylwester -- 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 Sun, Jul 21, 2013 at 12:22:48PM +0200, Sascha Hauer wrote: > On Sat, Jul 20, 2013 at 07:59:10PM -0700, Greg KH wrote: > > On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote: > > > On Sat, 20 Jul 2013, Greg KH wrote: > > > > > > > > >>That should be passed using platform data. > > > > > > > > > > > >Ick, don't pass strings around, pass pointers. If you have platform > > > > > >data you can get to, then put the pointer there, don't use a "name". > > > > > > > > > > I don't think I understood you here :-s We wont have phy pointer > > > > > when we create the device for the controller no?(it'll be done in > > > > > board file). Probably I'm missing something. > > > > > > > > Why will you not have that pointer? You can't rely on the "name" as the > > > > device id will not match up, so you should be able to rely on the > > > > pointer being in the structure that the board sets up, right? > > > > > > > > Don't use names, especially as ids can, and will, change, that is going > > > > to cause big problems. Use pointers, this is C, we are supposed to be > > > > doing that :) > > > > > > Kishon, I think what Greg means is this: The name you are using must > > > be stored somewhere in a data structure constructed by the board file, > > > right? Or at least, associated with some data structure somehow. > > > Otherwise the platform code wouldn't know which PHY hardware > > > corresponded to a particular name. > > > > > > Greg's suggestion is that you store the address of that data structure > > > in the platform data instead of storing the name string. Have the > > > consumer pass the data structure's address when it calls phy_create, > > > instead of passing the name. Then you don't have to worry about two > > > PHYs accidentally ending up with the same name or any other similar > > > problems. > > > > Close, but the issue is that whatever returns from phy_create() should > > then be used, no need to call any "find" functions, as you can just use > > the pointer that phy_create() returns. Much like all other class api > > functions in the kernel work. > > I think the problem here is to connect two from the bus structure > completely independent devices. Several frameworks (ASoC, soc-camera) > had this problem and this wasn't solved until the advent of devicetrees > and their phandles. > phy_create might be called from the probe function of some i2c device > (the phy device) and the resulting pointer is then needed in some other > platform devices (the user of the phy) probe function. > The best solution we have right now is implemented in the clk framework > which uses a string matching of the device names in clk_get() (at least > in the non-dt case). I would argue that clocks are wrong here as well, as others have already pointed out. What's wrong with the platform_data structure, why can't that be used for this? Or, if not, we can always add pointers to the platform device structure, or even the main 'struct device' as well, that's what it is there for. 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 Sun, Jul 21, 2013 at 01:12:07PM +0200, Tomasz Figa wrote:
> On Sunday 21 of July 2013 16:37:33 Kishon Vijay Abraham I wrote:
> > Hi,
> >
> > On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote:
> > > Hi,
> > >
> > > On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
> > >> On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
> > >>> On Sat, 20 Jul 2013, Greg KH wrote:
> > >>> That should be passed using platform data.
> > >>
> > >> Ick, don't pass strings around, pass pointers. If you have
> > >> platform
> > >> data you can get to, then put the pointer there, don't use a
> > >> "name".
> > >
> > > I don't think I understood you here :-s We wont have phy pointer
> > > when we create the device for the controller no?(it'll be done in
> > > board file). Probably I'm missing something.
> >
> > Why will you not have that pointer? You can't rely on the "name"
> > as
> > the device id will not match up, so you should be able to rely on
> > the pointer being in the structure that the board sets up, right?
> >
> > Don't use names, especially as ids can, and will, change, that is
> > going
> > to cause big problems. Use pointers, this is C, we are supposed to
> > be
> > doing that :)
> > >>>
> > >>> Kishon, I think what Greg means is this: The name you are using
> > >>> must
> > >>> be stored somewhere in a data structure constructed by the board
> > >>> file,
> > >>> right? Or at least, associated with some data structure somehow.
> > >>> Otherwise the platform code wouldn't know which PHY hardware
> > >>> corresponded to a particular name.
> > >>>
> > >>> Greg's suggestion is that you store the address of that data
> > >>> structure
> > >>> in the platform data instead of storing the name string. Have the
> > >>> consumer pass the data structure's address when it calls phy_create,
> > >>> instead of passing the name. Then you don't have to worry about two
> > >>> PHYs accidentally ending up with the same name or any other similar
> > >>> problems.
> > >>
> > >> Close, but the issue is that whatever returns from phy_create()
> > >> should
> > >> then be used, no need to call any "find" functions, as you can just
> > >> use
> > >> the pointer that phy_create() returns. Much like all other class api
> > >> functions in the kernel work.
> > >
> > > I think there is a confusion here about who registers the PHYs.
> > >
> > > All platform code does is registering a platform/i2c/whatever device,
> > > which causes a driver (located in drivers/phy/) to be instantiated.
> > > Such drivers call phy_create(), usually in their probe() callbacks,
> > > so platform_code has no way (and should have no way, for the sake of
> > > layering) to get what phy_create() returns.
Why not put pointers in the platform data structure that can hold these
pointers? I thought that is why we created those structures in the
first place. If not, what are they there for?
> > > IMHO we need a lookup method for PHYs, just like for clocks,
> > > regulators, PWMs or even i2c busses because there are complex cases
> > > when passing just a name using platform data will not work. I would
> > > second what Stephen said [1] and define a structure doing things in a
> > > DT-like way.
> > >
> > > Example;
> > >
> > > [platform code]
> > >
> > > static const struct phy_lookup my_phy_lookup[] = {
> > >
> > > PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
> >
> > The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
> > creating the device, the ids in the device name would change and
> > PHY_LOOKUP wont be useful.
>
> I don't think this is a problem. All the existing lookup methods already
> use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You
> can simply add a requirement that the ID must be assigned manually,
> without using PLATFORM_DEVID_AUTO to use PHY lookup.
And I'm saying that this idea, of using a specific name and id, is
frought with fragility and will break in the future in various ways when
devices get added to systems, making these strings constantly have to be
kept up to date with different board configurations.
People, NEVER, hardcode something like an id. The fact that this
happens today with the clock code, doesn't make it right, it makes the
clock code wrong. Others have already said that this is wrong there as
well, as systems change and dynamic ids get used more and more.
Let's not repeat the same mistakes of the past just because we refuse to
learn from them...
So again, the "find a phy by a string" functions should be removed, the
device id should be automatically created by the phy core just to make
things unique in sysfs, and no driver code should _ever_ be reliant on
the number that is being created, and the pointer to the phy structure
should be used everywhere instead.
With those types of changes, I will consider merging this subsystem, bu
Greetings from George Daniels
Greetings from George Daniels I am George Daniels, a Banker and credit system programmer (HSBC bank). I saw your email address while browsing through the bank D.T.C Screen in my office yesterday so I decided to use this very chance to know you. I believe we should use every opportunity to know each other better. However, I am contacting you for obvious reason which you will understand. I am sending this mail just to know if this email address is OK, reply me back so that I will send more details to you. I have a very important thing to discuss with you, I look forward to receiving your response at georgedani...@postino.net . Have a pleasant day. George Daniels -- 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
No events for LifeView FlyDVB-S LR300 (saa7134-input)
Hello, I have had a LifeView FlyDVB-S LR300 for many years and used it successfully with LIRC using the saa7134-input driver for much of that time. About a year ago, LIRC stopped working but I've never been able to determine precisely why. udev successfully symlinks my device to /dev/input/lr300 and lircd successfully starts using the -H devinput -d /dev/input/lr300 options. The problem is that the clients never seem to receive any events. I've tried really basic things like "ircat mythtv" with a single action defined, as shown below. Nothing. Not even a hint of activity with strace. begin prog = mythtv button = KEY_1 config = 1 end I know that the above configuration is being read from /etc/lirc/lircrc because I can see the contents in the strace output. My lircd.conf file is based on the top half of the sample devinput file distributed with LIRC, not the obsolete 32-bit bottom half. I know that is being read by lircd for the same reason. I did try creating my own with irrecord and while it was able to pick up the button presses, clients were subsequently unable to receive them. If I cat /dev/input/lr300 and press a few buttons then I see some output. However, this output stops after the first client connects. I then don't see any further output until I stop lircd. This may be normal, I'm not sure. I suspect it is because if I run lircd in the foreground with strace, I still see activity when I press buttons, even after the output from /dev/input/lr300 stops. This would lead one to suspect a communication issue between lircd and the clients but I do see the "accepted new client" message from lircd so there is at least some chatter going on. I'm not sure if these problems began with the arrival of the new remote control kernel drivers. I gather these make it possible to use remotes without the need for LIRC? I also gather that LIRC still has its uses and is well supported by applications so I still intend to use it. Either way, no matter what I do, I can never get any keytables or protocols associated with the device. ir-keytable always gives me this, despite having loaded all the associated modules. Found /sys/class/rc/rc0/ (/dev/input/event5) with: Driver saa7134, table rc-empty Supported protocols: Enabled protocols: Name: saa7134 IR (LifeView FlyDVB-S / bus: 1, vendor/product: 5168:0300, version: 0x0001 Repeat delay = 500 ms, repeat period = 125 ms Perhaps the saa7134-input driver hasn't been updated for this new model? Is this why I can't get LIRC to work? I actually believe these are two separate issues but please tell me if I'm wrong. My environment is a relatively up-to-date Gentoo system running Linux 3.9.4 and lirc 0.9.0. Any help would be appreciated and I will gladly provide more information. I did post this to the LIRC list initially but got no response there so trying here instead. Regards, James -- 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 Mauro It has given me the following error: 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> 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 dvb_frontend_event’ 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: 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 dvb_frontend_event’ 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: 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 dvb_frontend_event’ fs/compat_ioctl.c:1347:1: error: invalid application of ‘sizeof’ to incomplete type ‘struct dvb_frontend_event’ make[1]: *** [fs/compat_ioctl.o] Error 1 make: *** [fs] Error 2 alfredo@linux-puon:/usr/src/git/linux> --- What should I do now? I do not want experiment, since "bisect" is a very long process. Thank you, Alfredo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of
Re: Few Doubts on adding DT nodes for bridge driver
Hi Guennadi, On Sun, Jul 21, 2013 at 3:17 PM, Guennadi Liakhovetski wrote: > On Sun, 21 Jul 2013, Sylwester Nawrocki wrote: > [snip] >> > remote =<&tvp514x_2>; > > BTW, just occurred to me: shouldn't also these rather be > "remote-endpoint?" The documentation example should then be fixed too. > Ah correct I was referring the same and added in the 'remote' in device node. Shall I go ahead and post a patch fixing it ? >> > }; >> > }; >> > }; >> >> Are tvp514x@5c and tvp514x@5d decoders really connected to same bus, or are >> they on separate busses ? If the latter then you should have 2 'port' nodes. >> And in such case don't you need to identify to which >> >> > I have added two endpoints for the bridge driver. In the bridge driver >> > to build the pdata from DT node,I do the following, >> > >> > np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL); >> > >> > The above will give the first endpoint ie, endpoint@1 >> > From here is it possible to get the tvp514x_1 endpoint node and the >> > parent of it? >> >> Isn't v4l2_of_get_remote_port_parent() what you need ? > > Right, forgot we've got a helper for that already. > Yes this works for me now thank :) 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: Few Doubts on adding DT nodes for bridge driver
Hi Sylwester,
On Sun, Jul 21, 2013 at 2:54 PM, Sylwester Nawrocki
wrote:
> Hi Prabhakar,
>
>
> On 07/21/2013 08:20 AM, Prabhakar Lad wrote:
>>
>> Hi Sylwester, Guennadi,
>>
>> I am working on adding DT support for VPIF driver, initially to get
>> some hands dirty
>> on working on Capture driver and later will move ahead to add for the
>> display.
>> I have added asynchronous probing support for the both the bridge and
>> subdevs
>> which works perfectly like on a charm with passing pdata as usually,
>> but doing the
>> same with DT I have few doubts on building the pdata in the bridge driver.
>>
>>
>> This is a snippet of my subdes in i2c node:-
>>
>> i2c0: i2c@1c22000 {
>> status = "okay";
>> clock-frequency =<10>;
>> pinctrl-names = "default";
>> pinctrl-0 =<&i2c0_pins>;
>>
>> tvp514x@5c {
>> compatible = "ti,tvp5146";
>> reg =<0x5c>;
>>
>> port {
>> tvp514x_1: endpoint {
>> remote-endpoint
>> =<&vpif_capture0_1>;
>> hsync-active =<1>;
>> vsync-active =<1>;
>> pclk-sample =<0>;
>> };
>> };
>> };
>>
>> tvp514x@5d {
>> compatible = "ti,tvp5146";
>> reg =<0x5d>;
>>
>> port {
>> tvp514x_2: endpoint {
>> remote-endpoint
>> =<&vpif_capture0_0>;
>> hsync-active =<1>;
>> vsync-active =<1>;
>> pclk-sample =<0>;
>> };
>> };
>> };
>> ..
>> };
>>
>> Here tvp514x are the subdevs the platform has two of them one at 0x5c and
>> 0x5d,
>> so I have added two nodes for them.
>>
>> Following is DT node for the bridge driver:-
>>
>> vpif_capture@0 {
>> status = "okay";
>> port {
>
>
> You should also have:
> #address-cells = <1>;
> #size-cells = <0>;
>
> here or in vpif_capture node.
>
Ok
>
>> vpif_capture0_1: endpoint@1 {
>> remote =<&tvp514x_1>;
>> };
>> vpif_capture0_0: endpoint@0 {
>> remote =<&tvp514x_2>;
>> };
>> };
>> };
>
>
> Are tvp514x@5c and tvp514x@5d decoders really connected to same bus, or are
> they on separate busses ? If the latter then you should have 2 'port' nodes.
> And in such case don't you need to identify to which
>
The tvp514x@5c and tvp514x@5d are connected to the same bus.
>
>> I have added two endpoints for the bridge driver. In the bridge driver
>> to build the pdata from DT node,I do the following,
>>
>> np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL);
>>
>> The above will give the first endpoint ie, endpoint@1
>> From here is it possible to get the tvp514x_1 endpoint node and the
>> parent of it?
>
>
> Isn't v4l2_of_get_remote_port_parent() what you need ?
>
Al rite I'll check on it.
>
>> so that I build the asynchronous subdev list for the bridge driver.
>>
>>
>> +static struct v4l2_async_subdev tvp1_sd = {
>> + .hw = {
>
>
> This doesn't match the current struct v4l2_async_subdev data strcucture,
> there is no 'hw' field now.
>
>
Ah my bad pasted a wrong one earlier one :)
>> + .bus_type = V4L2_ASYNC_BUS_I2C,
>> + .match.i2c = {
>> + .adapter_id = 1,
>> + .address = 0x5c,
>> + },
>> + },
>> +};
>>
>> For building the asd subdev list in the bridge driver I can get the
>> address easily,
>> how do I get the adapter_id ? should this be a property subdev ? And also
>> same
>> with bustype.
>
>
> I had been working on the async subdev registration support in the
> exynos4-is
> driver this week and I have a few patches for v4l2-async.
> What those patches do is renaming V4L2_ASYNC_BUS_* to V4L2_ASYNC_MATCH_*,
> adding V4L2_ASYNC_MATCH_OF and a corresponding match_of callback like:
>
> static bool match_of(struct device *dev, struct v4l2_async_subdev *asd)
> {
> return dev->of_node == asd->match.of.node;
> }
>
> Then a driver registering the notifier, after parsing the device tree, can
> just pas
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Sunday 21 of July 2013 16:37:33 Kishon Vijay Abraham I wrote:
> Hi,
>
> On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote:
> > Hi,
> >
> > On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
> >> On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
> >>> On Sat, 20 Jul 2013, Greg KH wrote:
> >>> That should be passed using platform data.
> >>
> >> Ick, don't pass strings around, pass pointers. If you have
> >> platform
> >> data you can get to, then put the pointer there, don't use a
> >> "name".
> >
> > I don't think I understood you here :-s We wont have phy pointer
> > when we create the device for the controller no?(it'll be done in
> > board file). Probably I'm missing something.
>
> Why will you not have that pointer? You can't rely on the "name"
> as
> the device id will not match up, so you should be able to rely on
> the pointer being in the structure that the board sets up, right?
>
> Don't use names, especially as ids can, and will, change, that is
> going
> to cause big problems. Use pointers, this is C, we are supposed to
> be
> doing that :)
> >>>
> >>> Kishon, I think what Greg means is this: The name you are using
> >>> must
> >>> be stored somewhere in a data structure constructed by the board
> >>> file,
> >>> right? Or at least, associated with some data structure somehow.
> >>> Otherwise the platform code wouldn't know which PHY hardware
> >>> corresponded to a particular name.
> >>>
> >>> Greg's suggestion is that you store the address of that data
> >>> structure
> >>> in the platform data instead of storing the name string. Have the
> >>> consumer pass the data structure's address when it calls phy_create,
> >>> instead of passing the name. Then you don't have to worry about two
> >>> PHYs accidentally ending up with the same name or any other similar
> >>> problems.
> >>
> >> Close, but the issue is that whatever returns from phy_create()
> >> should
> >> then be used, no need to call any "find" functions, as you can just
> >> use
> >> the pointer that phy_create() returns. Much like all other class api
> >> functions in the kernel work.
> >
> > I think there is a confusion here about who registers the PHYs.
> >
> > All platform code does is registering a platform/i2c/whatever device,
> > which causes a driver (located in drivers/phy/) to be instantiated.
> > Such drivers call phy_create(), usually in their probe() callbacks,
> > so platform_code has no way (and should have no way, for the sake of
> > layering) to get what phy_create() returns.
>
> right.
>
> > IMHO we need a lookup method for PHYs, just like for clocks,
> > regulators, PWMs or even i2c busses because there are complex cases
> > when passing just a name using platform data will not work. I would
> > second what Stephen said [1] and define a structure doing things in a
> > DT-like way.
> >
> > Example;
> >
> > [platform code]
> >
> > static const struct phy_lookup my_phy_lookup[] = {
> >
> > PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
>
> The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
> creating the device, the ids in the device name would change and
> PHY_LOOKUP wont be useful.
I don't think this is a problem. All the existing lookup methods already
use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You
can simply add a requirement that the ID must be assigned manually,
without using PLATFORM_DEVID_AUTO to use PHY lookup.
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
Hi,
On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote:
Hi,
On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
On Sat, 20 Jul 2013, Greg KH wrote:
That should be passed using platform data.
Ick, don't pass strings around, pass pointers. If you have
platform
data you can get to, then put the pointer there, don't use a
"name".
I don't think I understood you here :-s We wont have phy pointer
when we create the device for the controller no?(it'll be done in
board file). Probably I'm missing something.
Why will you not have that pointer? You can't rely on the "name" as
the device id will not match up, so you should be able to rely on
the pointer being in the structure that the board sets up, right?
Don't use names, especially as ids can, and will, change, that is
going
to cause big problems. Use pointers, this is C, we are supposed to
be
doing that :)
Kishon, I think what Greg means is this: The name you are using must
be stored somewhere in a data structure constructed by the board file,
right? Or at least, associated with some data structure somehow.
Otherwise the platform code wouldn't know which PHY hardware
corresponded to a particular name.
Greg's suggestion is that you store the address of that data structure
in the platform data instead of storing the name string. Have the
consumer pass the data structure's address when it calls phy_create,
instead of passing the name. Then you don't have to worry about two
PHYs accidentally ending up with the same name or any other similar
problems.
Close, but the issue is that whatever returns from phy_create() should
then be used, no need to call any "find" functions, as you can just use
the pointer that phy_create() returns. Much like all other class api
functions in the kernel work.
I think there is a confusion here about who registers the PHYs.
All platform code does is registering a platform/i2c/whatever device,
which causes a driver (located in drivers/phy/) to be instantiated. Such
drivers call phy_create(), usually in their probe() callbacks, so
platform_code has no way (and should have no way, for the sake of
layering) to get what phy_create() returns.
right.
IMHO we need a lookup method for PHYs, just like for clocks, regulators,
PWMs or even i2c busses because there are complex cases when passing just
a name using platform data will not work. I would second what Stephen said
[1] and define a structure doing things in a DT-like way.
Example;
[platform code]
static const struct phy_lookup my_phy_lookup[] = {
PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
creating the device, the ids in the device name would change and
PHY_LOOKUP wont be useful.
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
Hi,
On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
> On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
> > On Sat, 20 Jul 2013, Greg KH wrote:
> > > > >>That should be passed using platform data.
> > > > >
> > > > >Ick, don't pass strings around, pass pointers. If you have
> > > > >platform
> > > > >data you can get to, then put the pointer there, don't use a
> > > > >"name".
> > > >
> > > > I don't think I understood you here :-s We wont have phy pointer
> > > > when we create the device for the controller no?(it'll be done in
> > > > board file). Probably I'm missing something.
> > >
> > > Why will you not have that pointer? You can't rely on the "name" as
> > > the device id will not match up, so you should be able to rely on
> > > the pointer being in the structure that the board sets up, right?
> > >
> > > Don't use names, especially as ids can, and will, change, that is
> > > going
> > > to cause big problems. Use pointers, this is C, we are supposed to
> > > be
> > > doing that :)
> >
> > Kishon, I think what Greg means is this: The name you are using must
> > be stored somewhere in a data structure constructed by the board file,
> > right? Or at least, associated with some data structure somehow.
> > Otherwise the platform code wouldn't know which PHY hardware
> > corresponded to a particular name.
> >
> > Greg's suggestion is that you store the address of that data structure
> > in the platform data instead of storing the name string. Have the
> > consumer pass the data structure's address when it calls phy_create,
> > instead of passing the name. Then you don't have to worry about two
> > PHYs accidentally ending up with the same name or any other similar
> > problems.
>
> Close, but the issue is that whatever returns from phy_create() should
> then be used, no need to call any "find" functions, as you can just use
> the pointer that phy_create() returns. Much like all other class api
> functions in the kernel work.
I think there is a confusion here about who registers the PHYs.
All platform code does is registering a platform/i2c/whatever device,
which causes a driver (located in drivers/phy/) to be instantiated. Such
drivers call phy_create(), usually in their probe() callbacks, so
platform_code has no way (and should have no way, for the sake of
layering) to get what phy_create() returns.
IMHO we need a lookup method for PHYs, just like for clocks, regulators,
PWMs or even i2c busses because there are complex cases when passing just
a name using platform data will not work. I would second what Stephen said
[1] and define a structure doing things in a DT-like way.
Example;
[platform code]
static const struct phy_lookup my_phy_lookup[] = {
PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1", "phy.2"),
/* etc... */
};
static void my_machine_init(void)
{
/* ... */
phy_register_lookup(my_phy_lookup, ARRAY_SIZE(my_phy_lookup));
/* ... */
}
/* Notice nothing stuffed in platform data. */
[provider code - samsung-usbphy driver]
static int samsung_usbphy_probe(...)
{
/* ... */
for (i = 0; i < PHY_COUNT; ++i) {
usbphy->phy[i].name = "phy";
usbphy->phy[i].id = i;
/* ... */
ret = phy_create(&usbphy->phy);
/* err handling */
}
/* ... */
}
[consumer code - s3c-hsotg driver]
static int s3c_hsotg_probe(...)
{
/* ... */
phy = phy_get(&pdev->dev, "otg");
/* ... */
}
[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813
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 Sat, Jul 20, 2013 at 07:59:10PM -0700, Greg KH wrote: > On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote: > > On Sat, 20 Jul 2013, Greg KH wrote: > > > > > > >>That should be passed using platform data. > > > > > > > > > >Ick, don't pass strings around, pass pointers. If you have platform > > > > >data you can get to, then put the pointer there, don't use a "name". > > > > > > > > I don't think I understood you here :-s We wont have phy pointer > > > > when we create the device for the controller no?(it'll be done in > > > > board file). Probably I'm missing something. > > > > > > Why will you not have that pointer? You can't rely on the "name" as the > > > device id will not match up, so you should be able to rely on the > > > pointer being in the structure that the board sets up, right? > > > > > > Don't use names, especially as ids can, and will, change, that is going > > > to cause big problems. Use pointers, this is C, we are supposed to be > > > doing that :) > > > > Kishon, I think what Greg means is this: The name you are using must > > be stored somewhere in a data structure constructed by the board file, > > right? Or at least, associated with some data structure somehow. > > Otherwise the platform code wouldn't know which PHY hardware > > corresponded to a particular name. > > > > Greg's suggestion is that you store the address of that data structure > > in the platform data instead of storing the name string. Have the > > consumer pass the data structure's address when it calls phy_create, > > instead of passing the name. Then you don't have to worry about two > > PHYs accidentally ending up with the same name or any other similar > > problems. > > Close, but the issue is that whatever returns from phy_create() should > then be used, no need to call any "find" functions, as you can just use > the pointer that phy_create() returns. Much like all other class api > functions in the kernel work. I think the problem here is to connect two from the bus structure completely independent devices. Several frameworks (ASoC, soc-camera) had this problem and this wasn't solved until the advent of devicetrees and their phandles. phy_create might be called from the probe function of some i2c device (the phy device) and the resulting pointer is then needed in some other platform devices (the user of the phy) probe function. The best solution we have right now is implemented in the clk framework which uses a string matching of the device names in clk_get() (at least in the non-dt case). Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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: Few Doubts on adding DT nodes for bridge driver
On Sun, 21 Jul 2013, Sylwester Nawrocki wrote:
> Hi Prabhakar,
>
> On 07/21/2013 08:20 AM, Prabhakar Lad wrote:
> > Hi Sylwester, Guennadi,
> >
> > I am working on adding DT support for VPIF driver, initially to get
> > some hands dirty
> > on working on Capture driver and later will move ahead to add for the
> > display.
> > I have added asynchronous probing support for the both the bridge and
> > subdevs
> > which works perfectly like on a charm with passing pdata as usually,
> > but doing the
> > same with DT I have few doubts on building the pdata in the bridge driver.
> >
> >
> > This is a snippet of my subdes in i2c node:-
> >
> > i2c0: i2c@1c22000 {
> > status = "okay";
> > clock-frequency =<10>;
> > pinctrl-names = "default";
> > pinctrl-0 =<&i2c0_pins>;
> >
> > tvp514x@5c {
> > compatible = "ti,tvp5146";
> > reg =<0x5c>;
> >
> > port {
> > tvp514x_1: endpoint {
> > remote-endpoint
> > =<&vpif_capture0_1>;
> > hsync-active =<1>;
> > vsync-active =<1>;
> > pclk-sample =<0>;
> > };
> > };
> > };
> >
> > tvp514x@5d {
> > compatible = "ti,tvp5146";
> > reg =<0x5d>;
> >
> > port {
> > tvp514x_2: endpoint {
> > remote-endpoint
> > =<&vpif_capture0_0>;
> > hsync-active =<1>;
> > vsync-active =<1>;
> > pclk-sample =<0>;
> > };
> > };
> > };
> > ..
> > };
> >
> > Here tvp514x are the subdevs the platform has two of them one at 0x5c and
> > 0x5d,
> > so I have added two nodes for them.
> >
> > Following is DT node for the bridge driver:-
> >
> > vpif_capture@0 {
> > status = "okay";
> > port {
>
> You should also have:
> #address-cells = <1>;
> #size-cells = <0>;
>
> here or in vpif_capture node.
>
> > vpif_capture0_1: endpoint@1 {
> > remote =<&tvp514x_1>;
> > };
> > vpif_capture0_0: endpoint@0 {
> > remote =<&tvp514x_2>;
BTW, just occurred to me: shouldn't also these rather be
"remote-endpoint?" The documentation example should then be fixed too.
> > };
> > };
> > };
>
> Are tvp514x@5c and tvp514x@5d decoders really connected to same bus, or are
> they on separate busses ? If the latter then you should have 2 'port' nodes.
> And in such case don't you need to identify to which
>
> > I have added two endpoints for the bridge driver. In the bridge driver
> > to build the pdata from DT node,I do the following,
> >
> > np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL);
> >
> > The above will give the first endpoint ie, endpoint@1
> > From here is it possible to get the tvp514x_1 endpoint node and the
> > parent of it?
>
> Isn't v4l2_of_get_remote_port_parent() what you need ?
Right, forgot we've got a helper for that already.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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: Few Doubts on adding DT nodes for bridge driver
Hi Prabhakar,
On 07/21/2013 08:20 AM, Prabhakar Lad wrote:
Hi Sylwester, Guennadi,
I am working on adding DT support for VPIF driver, initially to get
some hands dirty
on working on Capture driver and later will move ahead to add for the display.
I have added asynchronous probing support for the both the bridge and subdevs
which works perfectly like on a charm with passing pdata as usually,
but doing the
same with DT I have few doubts on building the pdata in the bridge driver.
This is a snippet of my subdes in i2c node:-
i2c0: i2c@1c22000 {
status = "okay";
clock-frequency =<10>;
pinctrl-names = "default";
pinctrl-0 =<&i2c0_pins>;
tvp514x@5c {
compatible = "ti,tvp5146";
reg =<0x5c>;
port {
tvp514x_1: endpoint {
remote-endpoint
=<&vpif_capture0_1>;
hsync-active =<1>;
vsync-active =<1>;
pclk-sample =<0>;
};
};
};
tvp514x@5d {
compatible = "ti,tvp5146";
reg =<0x5d>;
port {
tvp514x_2: endpoint {
remote-endpoint
=<&vpif_capture0_0>;
hsync-active =<1>;
vsync-active =<1>;
pclk-sample =<0>;
};
};
};
..
};
Here tvp514x are the subdevs the platform has two of them one at 0x5c and 0x5d,
so I have added two nodes for them.
Following is DT node for the bridge driver:-
vpif_capture@0 {
status = "okay";
port {
You should also have:
#address-cells = <1>;
#size-cells = <0>;
here or in vpif_capture node.
vpif_capture0_1: endpoint@1 {
remote =<&tvp514x_1>;
};
vpif_capture0_0: endpoint@0 {
remote =<&tvp514x_2>;
};
};
};
Are tvp514x@5c and tvp514x@5d decoders really connected to same bus, or are
they on separate busses ? If the latter then you should have 2 'port'
nodes.
And in such case don't you need to identify to which
I have added two endpoints for the bridge driver. In the bridge driver
to build the pdata from DT node,I do the following,
np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL);
The above will give the first endpoint ie, endpoint@1
From here is it possible to get the tvp514x_1 endpoint node and the
parent of it?
Isn't v4l2_of_get_remote_port_parent() what you need ?
so that I build the asynchronous subdev list for the bridge driver.
+static struct v4l2_async_subdev tvp1_sd = {
+ .hw = {
This doesn't match the current struct v4l2_async_subdev data strcucture,
there is no 'hw' field now.
+ .bus_type = V4L2_ASYNC_BUS_I2C,
+ .match.i2c = {
+ .adapter_id = 1,
+ .address = 0x5c,
+ },
+ },
+};
For building the asd subdev list in the bridge driver I can get the
address easily,
how do I get the adapter_id ? should this be a property subdev ? And also same
with bustype.
I had been working on the async subdev registration support in the
exynos4-is
driver this week and I have a few patches for v4l2-async.
What those patches do is renaming V4L2_ASYNC_BUS_* to V4L2_ASYNC_MATCH_*,
adding V4L2_ASYNC_MATCH_OF and a corresponding match_of callback like:
static bool match_of(struct device *dev, struct v4l2_async_subdev *asd)
{
return dev->of_node == asd->match.of.node;
}
Then a driver registering the notifier, after parsing the device tree, can
just pass a list of DT node pointers corresponding to its subdevs.
All this could also be achieved with V4L2_ASYNC_BUS_CUSTOM, but I think it's
better to make it as simple as possible for drivers by extending the core
a little.
I'm going to post those patches as RFC on Monday.
--
Regards,
Sylwester
--
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: Few Doubts on adding DT nodes for bridge driver
Hi Prabhakar
On Sun, 21 Jul 2013, Prabhakar Lad wrote:
> Hi Sylwester, Guennadi,
>
> I am working on adding DT support for VPIF driver, initially to get
> some hands dirty
> on working on Capture driver and later will move ahead to add for the display.
> I have added asynchronous probing support for the both the bridge and subdevs
> which works perfectly like on a charm with passing pdata as usually,
> but doing the
> same with DT I have few doubts on building the pdata in the bridge driver.
>
>
> This is a snippet of my subdes in i2c node:-
>
> i2c0: i2c@1c22000 {
> status = "okay";
> clock-frequency = <10>;
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins>;
>
> tvp514x@5c {
> compatible = "ti,tvp5146";
> reg = <0x5c>;
>
> port {
> tvp514x_1: endpoint {
> remote-endpoint =
> <&vpif_capture0_1>;
> hsync-active = <1>;
> vsync-active = <1>;
> pclk-sample = <0>;
> };
> };
> };
>
> tvp514x@5d {
> compatible = "ti,tvp5146";
> reg = <0x5d>;
>
> port {
> tvp514x_2: endpoint {
> remote-endpoint =
> <&vpif_capture0_0>;
> hsync-active = <1>;
> vsync-active = <1>;
> pclk-sample = <0>;
> };
> };
> };
> ..
> };
>
> Here tvp514x are the subdevs the platform has two of them one at 0x5c and
> 0x5d,
> so I have added two nodes for them.
>
> Following is DT node for the bridge driver:-
>
> vpif_capture@0 {
> status = "okay";
> port {
> vpif_capture0_1: endpoint@1 {
> remote = <&tvp514x_1>;
> };
> vpif_capture0_0: endpoint@0 {
> remote = <&tvp514x_2>;
> };
> };
> };
> I have added two endpoints for the bridge driver. In the bridge driver
> to build the pdata from DT node,I do the following,
>
> np = v4l2_of_get_next_endpoint(pdev->dev.of_node, NULL);
> The above will give the first endpoint ie, endpoint@1
> >From here is it possible to get the tvp514x_1 endpoint node and the
> parent of it?
Yes, you can use something like of_parse_phandle().
Thanks
Guennadi
> so that I build the asynchronous subdev list for the bridge driver.
>
>
> +static struct v4l2_async_subdev tvp1_sd = {
> + .hw = {
> + .bus_type = V4L2_ASYNC_BUS_I2C,
> + .match.i2c = {
> + .adapter_id = 1,
> + .address = 0x5c,
> + },
> + },
> +};
>
> For building the asd subdev list in the bridge driver I can get the
> address easily,
> how do I get the adapter_id ? should this be a property subdev ? And also same
> with bustype.
>
> Regards,
> --Prabhakar
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
