Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester, On Tuesday 31 July 2012 14:38:35 Sylwester Nawrocki wrote: > On 07/31/2012 01:05 PM, Laurent Pinchart wrote: > > What about CSI receivers that can reroute the lanes internally ? We > > would need to specify lane indices for each lane then, maybe with > > something like > > > > clock-lane =<0>; > > data-lanes =<2 3 1>; > > Sounds good to me. And the clock-lane could be made optional, as not > all devices would need it. > > However, as far as I can see, there is currently no generic API for > handling this kind of data structure. E.g. number of cells for the > "interrupts" property is specified with an additional > "#interrupt-cells" property. > > It would have been much easier to handle something like: > > data-lanes = <2>, <3>, <1>; > > i.e. an array of the lane indexes. > >>> > >>> I'm fine with that. > >> > >> ...on a second thought: shouldn't this be handled by pinctrl? Or is it > >> some CSI-2 module internal lane switching, not the global SoC pin > >> function configuration? > > > > On the hardware I came across, it's handled by the CSI2 receiver, not the > > SoC pinmux feature. > > Same here. Is there any hardware known to mux those MIPI-CSI > D-PHY high speed differential signals with general purpose IO pins ? > > Or are there mostly dedicated pins used ? The OMAP3 multiplexes the CSI pins with other functions. > However, if there are cases the lane configuration is done solely at CSI2 > receiver level, there seems little point in involving the pinctrl API. The OMAP3 handles lane routing in the CSI2 receiver. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
On 07/31/2012 01:05 PM, Laurent Pinchart wrote: > What about CSI receivers that can reroute the lanes internally ? We > would > need to specify lane indices for each lane then, maybe with something > like > > clock-lane =<0>; > data-lanes =<2 3 1>; Sounds good to me. And the clock-lane could be made optional, as not all devices would need it. However, as far as I can see, there is currently no generic API for handling this kind of data structure. E.g. number of cells for the "interrupts" property is specified with an additional "#interrupt-cells" property. It would have been much easier to handle something like: data-lanes = <2>, <3>, <1>; i.e. an array of the lane indexes. >>> >>> I'm fine with that. >> >> ...on a second thought: shouldn't this be handled by pinctrl? Or is it >> some CSI-2 module internal lane switching, not the global SoC pin function >> configuration? > > On the hardware I came across, it's handled by the CSI2 receiver, not the SoC > pinmux feature. Same here. Is there any hardware known to mux those MIPI-CSI D-PHY high speed differential signals with general purpose IO pins ? Or are there mostly dedicated pins used ? However, if there are cases the lane configuration is done solely at CSI2 receiver level, there seems little point in involving the pinctrl API. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Guennadi, On Tuesday 31 July 2012 12:58:50 Guennadi Liakhovetski wrote: > On Fri, 27 Jul 2012, Laurent Pinchart wrote: > > On Thursday 26 July 2012 21:51:30 Sylwester Nawrocki wrote: > > > On 07/26/2012 04:38 PM, Laurent Pinchart wrote: > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/video/mipi.txt > > > @@ -0,0 +1,5 @@ > > > +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and > > > transmitters > > > + > > > + - data-lanes : number of differential data lanes wired and > > > actively > > > used in > > > +communication between the transmitter and the receiver, > > > this > > > +excludes the clock lane; > > > >>> > > > >>> Wouldn't it be better to use the standard "bus-width" DT property? > > > >> > > > >> I can't see any problems with using "bus-width". It seems sufficient > > > >> and could indeed be better, without a need to invent new MIPI-CSI > > > >> specific names. That was my first RFC on that and my perspective > > > >> wasn't probably broad enough. :) > > > > > > > > What about CSI receivers that can reroute the lanes internally ? We > > > > would > > > > need to specify lane indices for each lane then, maybe with something > > > > like > > > > > > > > clock-lane =<0>; > > > > data-lanes =<2 3 1>; > > > > > > Sounds good to me. And the clock-lane could be made optional, as not all > > > devices would need it. > > > > > > However, as far as I can see, there is currently no generic API for > > > handling this kind of data structure. E.g. number of cells for the > > > "interrupts" property is specified with an additional > > > "#interrupt-cells" property. > > > > > > It would have been much easier to handle something like: > > > > > > data-lanes = <2>, <3>, <1>; > > > > > > i.e. an array of the lane indexes. > > > > I'm fine with that. > > ...on a second thought: shouldn't this be handled by pinctrl? Or is it > some CSI-2 module internal lane switching, not the global SoC pin function > configuration? On the hardware I came across, it's handled by the CSI2 receiver, not the SoC pinmux feature. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
On Fri, 27 Jul 2012, Laurent Pinchart wrote: > Hi Sylwester, > > On Thursday 26 July 2012 21:51:30 Sylwester Nawrocki wrote: > > On 07/26/2012 04:38 PM, Laurent Pinchart wrote: > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/video/mipi.txt > > @@ -0,0 +1,5 @@ > > +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and > > transmitters > > + > > + - data-lanes : number of differential data lanes wired and actively > > used in > > + communication between the transmitter and the receiver, > > this > > + excludes the clock lane; > > >>> > > >>> Wouldn't it be better to use the standard "bus-width" DT property? > > >> > > >> I can't see any problems with using "bus-width". It seems sufficient > > >> and could indeed be better, without a need to invent new MIPI-CSI > > >> specific names. That was my first RFC on that and my perspective > > >> wasn't probably broad enough. :) > > > > > > What about CSI receivers that can reroute the lanes internally ? We would > > > need to specify lane indices for each lane then, maybe with something > > > like > > > > > > clock-lane =<0>; > > > data-lanes =<2 3 1>; > > > > Sounds good to me. And the clock-lane could be made optional, as not all > > devices would need it. > > > > However, as far as I can see, there is currently no generic API for handling > > this kind of data structure. E.g. number of cells for the "interrupts" > > property is specified with an additional "#interrupt-cells" property. > > > > It would have been much easier to handle something like: > > > > data-lanes = <2>, <3>, <1>; > > > > i.e. an array of the lane indexes. > > I'm fine with that. ...on a second thought: shouldn't this be handled by pinctrl? Or is it some CSI-2 module internal lane switching, not the global SoC pin function configuration? Thanks Guennadi > > > For receivers that can't reroute lanes internally, the data-lanes property > > > would be need to specify lanes in sequence. > > > > > > data-lanes =<1 2 3>; > > > > In this case we would be only interested in the number of cells in this > > property, but how it could be retrieved ? With an array, it could have been > > calculated from property length returned by of_property_find() (divided by > > sizof(u32)). > > Agreed. > > -- > Regards, > > Laurent Pinchart > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
On Thu, 26 Jul 2012, Laurent Pinchart wrote:
> Hi Sylwester,
>
> On Tuesday 17 July 2012 20:16:23 Sylwester Nawrocki wrote:
> > On 07/16/2012 10:55 AM, Guennadi Liakhovetski wrote:
> > > Hi Sylwester
> > >
> > > Thanks for your comments to my RFC and for pointing out to this your
> > > earlier patch series. Unfortunately, I missed in in May, let me try to
> > > provide some thoughts about this, we should really try to converge our
> > > proposals. Maybe a discussion at KS would help too.
> >
> > Thank you for the review. I was happy to see your RFC, as previously
> > there seemed to be not much interest in DT among the media guys.
> > Certainly, we need to work on a common approach to ensure interoperability
> > of existing drivers and to avoid having people inventing different
> > bindings for common features. I would also expect some share of device
> > specific bindings, as diversity of media devices is significant.
> >
> > I'd be great to discuss these things at KS, especially support for
> > proper suspend/resume sequences. Also having common sessions with
> > other subsystems folks, like ASoC, for example, might be a good idea.
> >
> > I'm not sure if I'll be travelling to the KS though. :)
> >
> > > On Fri, 25 May 2012, Sylwester Nawrocki wrote:
> > >> s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
> > >> (camera host interface DMA engine and image processor). This patch
> > >> adds support for instantiating the MIPI-CSIS devices from DT and
> > >> parsing all SoC and board specific properties from device tree.
> > >> The MIPI DPHY control callback is now called directly from within
> > >> the driver, the platform code must ensure this callback does the
> > >> right thing for each SoC.
> > >>
> > >> The cell-index property is used to ensure proper signal routing,
> > >> from physical camera port, through MIPI-CSI2 receiver to the DMA
> > >> engine (FIMC?). It's also helpful in exposing the device topology
> > >> in user space at /dev/media? devnode (Media Controller API).
> > >>
> > >> This patch also defines a common property ("data-lanes") for MIPI-CSI
> > >> receivers and transmitters.
> > >>
> > >> Signed-off-by: Sylwester Nawrocki
> > >> Signed-off-by: Kyungmin Park
> > >> ---
> > >>
> > >> Documentation/devicetree/bindings/video/mipi.txt |5 +
> > >> .../bindings/video/samsung-mipi-csis.txt | 47 ++
> > >> drivers/media/video/s5p-fimc/mipi-csis.c | 97
> > >> +++- drivers/media/video/s5p-fimc/mipi-csis.h
> > >>|1 +
> > >> 4 files changed, 126 insertions(+), 24 deletions(-)
> > >> create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
> > >> create mode 100644
> > >> Documentation/devicetree/bindings/video/samsung-mipi-csis.txt>>
> > >> diff --git a/Documentation/devicetree/bindings/video/mipi.txt
> > >> b/Documentation/devicetree/bindings/video/mipi.txt new file mode 100644
> > >> index 000..5aed285
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/video/mipi.txt
> > >> @@ -0,0 +1,5 @@
> > >> +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
> > >> +
> > >> + - data-lanes : number of differential data lanes wired and actively
> > >> used in
> > >> +communication between the transmitter and the receiver,
> > >> this
> > >> +excludes the clock lane;
> > >
> > > Wouldn't it be better to use the standard "bus-width" DT property?
> >
> > I can't see any problems with using "bus-width". It seems sufficient
> > and could indeed be better, without a need to invent new MIPI-CSI
> > specific names. That was my first RFC on that and my perspective
> > wasn't probably broad enough. :)
>
> What about CSI receivers that can reroute the lanes internally ? We would
> need
> to specify lane indices for each lane then, maybe with something like
>
> clock-lane = <0>;
"clock-lanes" for uniformity?
> data-lanes = <2 3 1>;
Are there transmitters, that can reroute lanes too?
Thanks
Guennadi
> For receivers that can't reroute lanes internally, the data-lanes property
> would be need to specify lanes in sequence.
>
> data-lanes = <1 2 3>;
>
> --
> Regards,
>
> Laurent Pinchart
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester, On Thursday 26 July 2012 21:51:30 Sylwester Nawrocki wrote: > On 07/26/2012 04:38 PM, Laurent Pinchart wrote: > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/mipi.txt > @@ -0,0 +1,5 @@ > +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and > transmitters > + > + - data-lanes : number of differential data lanes wired and actively > used in > +communication between the transmitter and the receiver, > this > +excludes the clock lane; > >>> > >>> Wouldn't it be better to use the standard "bus-width" DT property? > >> > >> I can't see any problems with using "bus-width". It seems sufficient > >> and could indeed be better, without a need to invent new MIPI-CSI > >> specific names. That was my first RFC on that and my perspective > >> wasn't probably broad enough. :) > > > > What about CSI receivers that can reroute the lanes internally ? We would > > need to specify lane indices for each lane then, maybe with something > > like > > > > clock-lane =<0>; > > data-lanes =<2 3 1>; > > Sounds good to me. And the clock-lane could be made optional, as not all > devices would need it. > > However, as far as I can see, there is currently no generic API for handling > this kind of data structure. E.g. number of cells for the "interrupts" > property is specified with an additional "#interrupt-cells" property. > > It would have been much easier to handle something like: > > data-lanes = <2>, <3>, <1>; > > i.e. an array of the lane indexes. I'm fine with that. > > For receivers that can't reroute lanes internally, the data-lanes property > > would be need to specify lanes in sequence. > > > > data-lanes =<1 2 3>; > > In this case we would be only interested in the number of cells in this > property, but how it could be retrieved ? With an array, it could have been > calculated from property length returned by of_property_find() (divided by > sizof(u32)). Agreed. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Laurent, On 07/26/2012 04:38 PM, Laurent Pinchart wrote: --- /dev/null +++ b/Documentation/devicetree/bindings/video/mipi.txt @@ -0,0 +1,5 @@ +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters + + - data-lanes : number of differential data lanes wired and actively used in + communication between the transmitter and the receiver, this + excludes the clock lane; >>> >>> Wouldn't it be better to use the standard "bus-width" DT property? >> >> I can't see any problems with using "bus-width". It seems sufficient >> and could indeed be better, without a need to invent new MIPI-CSI >> specific names. That was my first RFC on that and my perspective >> wasn't probably broad enough. :) > > What about CSI receivers that can reroute the lanes internally ? We would need > to specify lane indices for each lane then, maybe with something like > > clock-lane =<0>; > data-lanes =<2 3 1>; Sounds good to me. And the clock-lane could be made optional, as not all devices would need it. However, as far as I can see, there is currently no generic API for handling this kind of data structure. E.g. number of cells for the "interrupts" property is specified with an additional "#interrupt-cells" property. It would have been much easier to handle something like: data-lanes = <2>, <3>, <1>; i.e. an array of the lane indexes. > For receivers that can't reroute lanes internally, the data-lanes property > would be need to specify lanes in sequence. > > data-lanes =<1 2 3>; In this case we would be only interested in the number of cells in this property, but how it could be retrieved ? With an array, it could have been calculated from property length returned by of_property_find() (divided by sizof(u32)). -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester,
On Tuesday 17 July 2012 20:16:23 Sylwester Nawrocki wrote:
> On 07/16/2012 10:55 AM, Guennadi Liakhovetski wrote:
> > Hi Sylwester
> >
> > Thanks for your comments to my RFC and for pointing out to this your
> > earlier patch series. Unfortunately, I missed in in May, let me try to
> > provide some thoughts about this, we should really try to converge our
> > proposals. Maybe a discussion at KS would help too.
>
> Thank you for the review. I was happy to see your RFC, as previously
> there seemed to be not much interest in DT among the media guys.
> Certainly, we need to work on a common approach to ensure interoperability
> of existing drivers and to avoid having people inventing different
> bindings for common features. I would also expect some share of device
> specific bindings, as diversity of media devices is significant.
>
> I'd be great to discuss these things at KS, especially support for
> proper suspend/resume sequences. Also having common sessions with
> other subsystems folks, like ASoC, for example, might be a good idea.
>
> I'm not sure if I'll be travelling to the KS though. :)
>
> > On Fri, 25 May 2012, Sylwester Nawrocki wrote:
> >> s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
> >> (camera host interface DMA engine and image processor). This patch
> >> adds support for instantiating the MIPI-CSIS devices from DT and
> >> parsing all SoC and board specific properties from device tree.
> >> The MIPI DPHY control callback is now called directly from within
> >> the driver, the platform code must ensure this callback does the
> >> right thing for each SoC.
> >>
> >> The cell-index property is used to ensure proper signal routing,
> >> from physical camera port, through MIPI-CSI2 receiver to the DMA
> >> engine (FIMC?). It's also helpful in exposing the device topology
> >> in user space at /dev/media? devnode (Media Controller API).
> >>
> >> This patch also defines a common property ("data-lanes") for MIPI-CSI
> >> receivers and transmitters.
> >>
> >> Signed-off-by: Sylwester Nawrocki
> >> Signed-off-by: Kyungmin Park
> >> ---
> >>
> >> Documentation/devicetree/bindings/video/mipi.txt |5 +
> >> .../bindings/video/samsung-mipi-csis.txt | 47 ++
> >> drivers/media/video/s5p-fimc/mipi-csis.c | 97
> >> +++- drivers/media/video/s5p-fimc/mipi-csis.h
> >>|1 +
> >> 4 files changed, 126 insertions(+), 24 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
> >> create mode 100644
> >> Documentation/devicetree/bindings/video/samsung-mipi-csis.txt>>
> >> diff --git a/Documentation/devicetree/bindings/video/mipi.txt
> >> b/Documentation/devicetree/bindings/video/mipi.txt new file mode 100644
> >> index 000..5aed285
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/video/mipi.txt
> >> @@ -0,0 +1,5 @@
> >> +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
> >> +
> >> + - data-lanes : number of differential data lanes wired and actively
> >> used in
> >> + communication between the transmitter and the receiver, this
> >> + excludes the clock lane;
> >
> > Wouldn't it be better to use the standard "bus-width" DT property?
>
> I can't see any problems with using "bus-width". It seems sufficient
> and could indeed be better, without a need to invent new MIPI-CSI
> specific names. That was my first RFC on that and my perspective
> wasn't probably broad enough. :)
What about CSI receivers that can reroute the lanes internally ? We would need
to specify lane indices for each lane then, maybe with something like
clock-lane = <0>;
data-lanes = <2 3 1>;
For receivers that can't reroute lanes internally, the data-lanes property
would be need to specify lanes in sequence.
data-lanes = <1 2 3>;
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Guennadi,
On 07/16/2012 10:55 AM, Guennadi Liakhovetski wrote:
> Hi Sylwester
>
> Thanks for your comments to my RFC and for pointing out to this your
> earlier patch series. Unfortunately, I missed in in May, let me try to
> provide some thoughts about this, we should really try to converge our
> proposals. Maybe a discussion at KS would help too.
Thank you for the review. I was happy to see your RFC, as previously
there seemed to be not much interest in DT among the media guys.
Certainly, we need to work on a common approach to ensure interoperability
of existing drivers and to avoid having people inventing different
bindings for common features. I would also expect some share of device
specific bindings, as diversity of media devices is significant.
I'd be great to discuss these things at KS, especially support for
proper suspend/resume sequences. Also having common sessions with
other subsystems folks, like ASoC, for example, might be a good idea.
I'm not sure if I'll be travelling to the KS though. :)
> On Fri, 25 May 2012, Sylwester Nawrocki wrote:
>
>> s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
>> (camera host interface DMA engine and image processor). This patch
>> adds support for instantiating the MIPI-CSIS devices from DT and
>> parsing all SoC and board specific properties from device tree.
>> The MIPI DPHY control callback is now called directly from within
>> the driver, the platform code must ensure this callback does the
>> right thing for each SoC.
>>
>> The cell-index property is used to ensure proper signal routing,
>> from physical camera port, through MIPI-CSI2 receiver to the DMA
>> engine (FIMC?). It's also helpful in exposing the device topology
>> in user space at /dev/media? devnode (Media Controller API).
>>
>> This patch also defines a common property ("data-lanes") for MIPI-CSI
>> receivers and transmitters.
>>
>> Signed-off-by: Sylwester Nawrocki
>> Signed-off-by: Kyungmin Park
>> ---
>> Documentation/devicetree/bindings/video/mipi.txt |5 +
>> .../bindings/video/samsung-mipi-csis.txt | 47 ++
>> drivers/media/video/s5p-fimc/mipi-csis.c | 97
>> +++-
>> drivers/media/video/s5p-fimc/mipi-csis.h |1 +
>> 4 files changed, 126 insertions(+), 24 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
>> create mode 100644
>> Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
>>
>> diff --git a/Documentation/devicetree/bindings/video/mipi.txt
>> b/Documentation/devicetree/bindings/video/mipi.txt
>> new file mode 100644
>> index 000..5aed285
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/mipi.txt
>> @@ -0,0 +1,5 @@
>> +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
>> +
>> + - data-lanes : number of differential data lanes wired and actively used in
>> +communication between the transmitter and the receiver, this
>> +excludes the clock lane;
>
> Wouldn't it be better to use the standard "bus-width" DT property?
I can't see any problems with using "bus-width". It seems sufficient
and could indeed be better, without a need to invent new MIPI-CSI
specific names. That was my first RFC on that and my perspective
wasn't probably broad enough. :)
>> diff --git a/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
>> b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
>> new file mode 100644
>> index 000..7bce6f4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
>> @@ -0,0 +1,47 @@
>> +Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS)
>> +-
>> +
>> +Required properties:
>> +
>> +- compatible - one of :
>> +"samsung,s5pv210-csis",
>> +"samsung,exynos4210-csis",
>> +"samsung,exynos4212-csis",
>> +"samsung,exynos4412-csis";
>> +- reg : physical base address and size of the device memory mapped
>> registers;
>> +- interrupts : should contain MIPI CSIS interrupt; the format of the
>> +interrupt specifier depends on the interrupt controller;
>> +- cell-index : the hardware instance index;
>
> Not sure whether this is absolutely needed... Wouldn't it be sufficient to
> just enumerate them during probing?
As Grant pointed to me, the "cell-index" property is something that we should
be avoiding. But I needed something like this in the driver,
to differentiate between the multiple IP instances. I cannot simply assign
the indexes in random way to the hardware instances. Each of the MIPI-CSI
Slaves has different properties (e.g. supporting max. 2 or 4 data lanes).
Additionally, a particular MIPI-CSI Slave instance is hard-wired inside the
SoC to the FIMC input mux (cross-bar) and physical video port. IOW, an image
sensor/video port/MIPI-CSIS instance assignment is fixed. If two MIP
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester
Thanks for your comments to my RFC and for pointing out to this your
earlier patch series. Unfortunately, I missed in in May, let me try to
provide some thoughts about this, we should really try to converge our
proposals. Maybe a discussion at KS would help too.
On Fri, 25 May 2012, Sylwester Nawrocki wrote:
> s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
> (camera host interface DMA engine and image processor). This patch
> adds support for instantiating the MIPI-CSIS devices from DT and
> parsing all SoC and board specific properties from device tree.
> The MIPI DPHY control callback is now called directly from within
> the driver, the platform code must ensure this callback does the
> right thing for each SoC.
>
> The cell-index property is used to ensure proper signal routing,
> from physical camera port, through MIPI-CSI2 receiver to the DMA
> engine (FIMC?). It's also helpful in exposing the device topology
> in user space at /dev/media? devnode (Media Controller API).
>
> This patch also defines a common property ("data-lanes") for MIPI-CSI
> receivers and transmitters.
>
> Signed-off-by: Sylwester Nawrocki
> Signed-off-by: Kyungmin Park
> ---
> Documentation/devicetree/bindings/video/mipi.txt |5 +
> .../bindings/video/samsung-mipi-csis.txt | 47 ++
> drivers/media/video/s5p-fimc/mipi-csis.c | 97
> +++-
> drivers/media/video/s5p-fimc/mipi-csis.h |1 +
> 4 files changed, 126 insertions(+), 24 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
> create mode 100644
> Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
>
> diff --git a/Documentation/devicetree/bindings/video/mipi.txt
> b/Documentation/devicetree/bindings/video/mipi.txt
> new file mode 100644
> index 000..5aed285
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/mipi.txt
> @@ -0,0 +1,5 @@
> +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
> +
> + - data-lanes : number of differential data lanes wired and actively used in
> + communication between the transmitter and the receiver, this
> + excludes the clock lane;
Wouldn't it be better to use the standard "bus-width" DT property?
> diff --git a/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
> b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
> new file mode 100644
> index 000..7bce6f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
> @@ -0,0 +1,47 @@
> +Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS)
> +-
> +
> +Required properties:
> +
> +- compatible - one of :
> + "samsung,s5pv210-csis",
> + "samsung,exynos4210-csis",
> + "samsung,exynos4212-csis",
> + "samsung,exynos4412-csis";
> +- reg : physical base address and size of the device memory mapped registers;
> +- interrupts : should contain MIPI CSIS interrupt; the format of the
> + interrupt specifier depends on the interrupt controller;
> +- cell-index : the hardware instance index;
Not sure whether this is absolutely needed... Wouldn't it be sufficient to
just enumerate them during probing?
> +- clock-frequency : The IP's main (system bus) clock frequency in Hz, the
> default
> + value when this property is not specified is 166 MHz;
> +- data-lanes : number of physical MIPI-CSI2 lanes used;
ditto - bus-width?
> +- samsung,csis-hs-settle : differential receiver (HS-RX) settle time;
> +- vddio-supply: MIPI CSIS I/O and PLL voltage supply (e.g. 1.8V);
> +- vddcore-supply : MIPI CSIS Core voltage supply (e.g. 1.1V).
> +
> +Example:
> +
> + reg0: regulator@0 {
> + };
> +
> + reg1: regulator@1 {
> + };
> +
> +/* SoC properties */
> +
> + csis@1188 {
> + compatible = "samsung,exynos4210-csis";
> + reg = <0x1188 0x1000>;
> + interrupts = <0 78 0>;
> + cell-index = <0>;
> + };
> +
> +/* Board properties */
> +
> + csis@1188 {
> + clock-frequency = <16600>;
> + data-lanes = <2>;
> + samsung,csis-hs-settle = <12>;
> + vddio-supply = <®0>;
> + vddcore-supply = <®1>;
> + };
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-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 02/13] media: s5p-csis: Add device tree support
s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
(camera host interface DMA engine and image processor). This patch
adds support for instantiating the MIPI-CSIS devices from DT and
parsing all SoC and board specific properties from device tree.
The MIPI DPHY control callback is now called directly from within
the driver, the platform code must ensure this callback does the
right thing for each SoC.
The cell-index property is used to ensure proper signal routing,
from physical camera port, through MIPI-CSI2 receiver to the DMA
engine (FIMC?). It's also helpful in exposing the device topology
in user space at /dev/media? devnode (Media Controller API).
This patch also defines a common property ("data-lanes") for MIPI-CSI
receivers and transmitters.
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
Documentation/devicetree/bindings/video/mipi.txt |5 +
.../bindings/video/samsung-mipi-csis.txt | 47 ++
drivers/media/video/s5p-fimc/mipi-csis.c | 97 +++-
drivers/media/video/s5p-fimc/mipi-csis.h |1 +
4 files changed, 126 insertions(+), 24 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
create mode 100644
Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
diff --git a/Documentation/devicetree/bindings/video/mipi.txt
b/Documentation/devicetree/bindings/video/mipi.txt
new file mode 100644
index 000..5aed285
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/mipi.txt
@@ -0,0 +1,5 @@
+Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
+
+ - data-lanes : number of differential data lanes wired and actively used in
+ communication between the transmitter and the receiver, this
+ excludes the clock lane;
diff --git a/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
new file mode 100644
index 000..7bce6f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
@@ -0,0 +1,47 @@
+Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS)
+-
+
+Required properties:
+
+- compatible - one of :
+ "samsung,s5pv210-csis",
+ "samsung,exynos4210-csis",
+ "samsung,exynos4212-csis",
+ "samsung,exynos4412-csis";
+- reg : physical base address and size of the device memory mapped registers;
+- interrupts : should contain MIPI CSIS interrupt; the format of the
+ interrupt specifier depends on the interrupt controller;
+- cell-index : the hardware instance index;
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, the
default
+ value when this property is not specified is 166 MHz;
+- data-lanes : number of physical MIPI-CSI2 lanes used;
+- samsung,csis-hs-settle : differential receiver (HS-RX) settle time;
+- vddio-supply: MIPI CSIS I/O and PLL voltage supply (e.g. 1.8V);
+- vddcore-supply : MIPI CSIS Core voltage supply (e.g. 1.1V).
+
+Example:
+
+ reg0: regulator@0 {
+ };
+
+ reg1: regulator@1 {
+ };
+
+/* SoC properties */
+
+ csis@1188 {
+ compatible = "samsung,exynos4210-csis";
+ reg = <0x1188 0x1000>;
+ interrupts = <0 78 0>;
+ cell-index = <0>;
+ };
+
+/* Board properties */
+
+ csis@1188 {
+ clock-frequency = <16600>;
+ data-lanes = <2>;
+ samsung,csis-hs-settle = <12>;
+ vddio-supply = <®0>;
+ vddcore-supply = <®1>;
+ };
diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c
b/drivers/media/video/s5p-fimc/mipi-csis.c
index 2f73d9e..ffb820e 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -80,10 +81,11 @@ static char *csi_clock_name[] = {
[CSIS_CLK_GATE] = "csis",
};
#define NUM_CSIS_CLOCKSARRAY_SIZE(csi_clock_name)
+#define DEFAULT_SCLK_CSIS_FREQ 16600UL
static const char * const csis_supply_name[] = {
- "vdd11", /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */
- "vdd18", /* VDD 1.8V and MIPI CSI PLL supply */
+ "vddcore", /* CSIS Core (1.0V, 1.1V or 1.2V) suppply */
+ "vddio",/* CSIS I/O and PLL (1.8V) supply */
};
#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name)
@@ -99,11 +101,15 @@ enum {
*protecting @format and @flags members
* @pads: CSIS pads array
* @sd: v4l2_subdev associated with CSIS device instance
+ * @index: the hardware instance index
* @pdev: CSIS platform device
* @regs: mmaped I/O registers memory
* @clock: CSIS clocks
* @irq: requested s5p-mipi-csis irq number
* @flags: the state variable for
