Re: [PATCH v3 6/7] dt-bindings: media: Add Renesas R-Car DRIF binding

[Laurent Pinchart]

Hello,

On Thursday 16 Feb 2017 11:02:55 Ramesh Shanmugasundaram wrote:
> Hi Rob,
> 
> Thank you for the review comments.
> 
> > Subject: Re: [PATCH v3 6/7] dt-bindings: media: Add Renesas R-Car DRIF
> > binding
> > 
> > On Tue, Feb 07, 2017 at 03:02:36PM +, Ramesh Shanmugasundaram wrote:
> >> Add binding documentation for Renesas R-Car Digital Radio Interface
> >> (DRIF) controller.
> >> 
> >> Signed-off-by: Ramesh Shanmugasundaram
> >> 
> >> ---
> >> 
> >>  .../devicetree/bindings/media/renesas,drif.txt | 186 ++
> >>  1 file changed, 186 insertions(+)
> >>  create mode 100644
> >> 
> >> Documentation/devicetree/bindings/media/renesas,drif.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
> >> b/Documentation/devicetree/bindings/media/renesas,drif.txt
> >> new file mode 100644
> >> index 000..6315609
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> >> @@ -0,0 +1,186 @@
> >> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> >> +
> >> +
> >> +R-Car Gen3 DRIF is a SPI like receive only slave device. A general
> >> +representation of DRIF interfacing with a master device is shown below.
> >> +
> >> ++-++-+
> >> +| |-SCK--->|CLK  |
> >> +|   Master|-SS>|SYNC  DRIFn (slave)  |
> >> +| |-SD0--->|D0   |
> >> +| |-SD1--->|D1   |
> >> ++-++-+
> >> +
> >> +As per the datasheet, each DRIF channel (drifn) is made up of two
> >> +internal channels (drifn0 & drifn1). These two internal channels
> >> +share the common CLK & SYNC. Each internal channel has its own
> >> +dedicated resources like irq, dma channels, address space & clock.
> >> +This internal split is not visible to the external master device.
> >> +
> >> +The device tree model represents each internal channel as a separate
> >> node.
> >> +The internal channels sharing the CLK & SYNC are tied together by
> >> +their phandles using a new property called "renesas,bonding". For the
> >> +rest of the documentation, unless explicitly stated, the word channel
> >> +implies an internal channel.
> >> +
> >> +When both internal channels are enabled they need to be managed
> >> +together as one (i.e.) they cannot operate alone as independent
> >> +devices. Out of the two, one of them needs to act as a primary device
> >> +that accepts common properties of both the internal channels. This
> >> +channel is identified by a new property called "renesas,primary-bond".
> >> +
> >> +To summarize,
> >> +   - When both the internal channels that are bonded together are
> >> enabled,
> >> + the zeroth channel is selected as primary-bond. This channels
> >> accepts
> >> + properties common to all the members of the bond.
> >> +   - When only one of the bonded channels need to be enabled, the
> >> property
> >> + "renesas,bonding" or "renesas,primary-bond" will have no effect.
> >> That
> >> + enabled channel can act alone as any other independent device.
> >> +
> >> +Required properties of an internal channel:
> >> +---
> >> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of
> >> R8A7795 SoC.
> >> +"renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible
> >> device.
> >> +When compatible with the generic version, nodes must list the
> >> +SoC-specific version corresponding to the platform first
> >> +followed by the generic version.
> >> +- reg: offset and length of that channel.
> >> +- interrupts: associated with that channel.
> >> +- clocks: phandle and clock specifier of that channel.
> >> +- clock-names: clock input name string: "fck".
> >> +- dmas: phandles to the DMA channels.
> >> +- dma-names: names of the DMA channel: "rx".
> >> +- renesas,bonding: phandle to the other channel.
> >> +
> >> +Optional properties of an internal channel:
> >> +---
> >> +- power-domains: phandle to the respective power domain.
> >> +
> >> +Required properties of an internal channel when:
> >> +  - It is the only enabled channel of the bond (or)
> >> +  - If it acts as primary among enabled bonds
> >> +
> >> +- pinctrl-0: pin control group to be used for this channel.
> >> +- pinctrl-names: must be "default".
> >> +- renesas,primary-bond: empty property indicating the channel acts as
> >> primary
> >> +  among the bonded channels.
> >> +- port: child port node of a channel that defines the local and remote
> >> +  endpoints. The remote endpoint is assumed to be a third party tuner
> >> +  device 

RE: [PATCH 0/3] usb: host: xhci-plat: add support suspend/resume for R-Car

[Yoshihiro Shimoda]

Hi Mathias,

> From: Yoshihiro Shimoda, Sent: Friday, March 17, 2017 7:03 PM
> 
> This patch adds support suspend/resume for R-Car controllers.
> The controllers need firmware downloading and the current code has
> such process only when probe timing. After suspend the system and
> the power of controller down, the driver needs to re-download the firmware
> in resume timing. This patch adds such a process.
> Also, since previous xhci-plat code didnn't handle the clk in suspend/resume,
> this patch set has such process.

I'm afraid but would you review this patch set?
This can be applied on the latest Greg's usb.git / usb-next branch at the 
moment.

Best regards,
Yoshihiro Shimoda

> Yoshihiro Shimoda (3):
>   usb: host: xhci-plat: enable clk in resume timing
>   usb: host: xhci-plat: add resume_quirk()
>   usb: host: xhci-plat: set resume_quirk() for R-Car controllers
> 
>  drivers/usb/host/xhci-plat.c | 29 -
>  drivers/usb/host/xhci-plat.h |  1 +
>  drivers/usb/host/xhci-rcar.c | 11 +++
>  drivers/usb/host/xhci-rcar.h |  6 ++
>  4 files changed, 46 insertions(+), 1 deletion(-)
> 
> --
> 1.9.1

Re: [PATCH v3 6/7] dt-bindings: media: Add Renesas R-Car DRIF binding

[Laurent Pinchart]

Hi Ramesh,

Thank you for the patch.

On Tuesday 07 Feb 2017 15:02:36 Ramesh Shanmugasundaram wrote:
> Add binding documentation for Renesas R-Car Digital Radio Interface
> (DRIF) controller.
> 
> Signed-off-by: Ramesh Shanmugasundaram
> 
> ---
>  .../devicetree/bindings/media/renesas,drif.txt | 186 ++
>  1 file changed, 186 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file mode
> 100644
> index 000..6315609
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> @@ -0,0 +1,186 @@
> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> +
> +
> +R-Car Gen3 DRIF is a SPI like receive only slave device. A general
> +representation of DRIF interfacing with a master device is shown below.
> +
> ++-++-+
> +| |-SCK--->|CLK  |
> +|   Master|-SS>|SYNC  DRIFn (slave)  |
> +| |-SD0--->|D0   |
> +| |-SD1--->|D1   |
> ++-++-+
> +
> +As per the datasheet, each DRIF channel (drifn) is made up of two internal
> +channels (drifn0 & drifn1). These two internal channels share the common
> +CLK & SYNC. Each internal channel has its own dedicated resources like
> +irq, dma channels, address space & clock. This internal split is not
> +visible to the external master device.
> +
> +The device tree model represents each internal channel as a separate node.
> +The internal channels sharing the CLK & SYNC are tied together by their
> +phandles using a new property called "renesas,bonding". For the rest of
> +the documentation, unless explicitly stated, the word channel implies an
> +internal channel.
> +
> +When both internal channels are enabled they need to be managed together
> +as one (i.e.) they cannot operate alone as independent devices. Out of the
> +two, one of them needs to act as a primary device that accepts common
> +properties of both the internal channels. This channel is identified by a
> +new property called "renesas,primary-bond".
> +
> +To summarize,
> +   - When both the internal channels that are bonded together are enabled,
> + the zeroth channel is selected as primary-bond. This channels accepts
> + properties common to all the members of the bond.
> +   - When only one of the bonded channels need to be enabled, the property
> + "renesas,bonding" or "renesas,primary-bond" will have no effect. That
> + enabled channel can act alone as any other independent device.
> +
> +Required properties of an internal channel:
> +---
> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of
> R8A7795 SoC.
> +   "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible
> device.
> +   When compatible with the generic version, nodes must list the
> +   SoC-specific version corresponding to the platform first
> +   followed by the generic version.
> +- reg: offset and length of that channel.
> +- interrupts: associated with that channel.
> +- clocks: phandle and clock specifier of that channel.
> +- clock-names: clock input name string: "fck".
> +- dmas: phandles to the DMA channels.
> +- dma-names: names of the DMA channel: "rx".
> +- renesas,bonding: phandle to the other channel.
> +
> +Optional properties of an internal channel:
> +---
> +- power-domains: phandle to the respective power domain.
> +
> +Required properties of an internal channel when:
> + - It is the only enabled channel of the bond (or)
> + - If it acts as primary among enabled bonds
> +
> +- pinctrl-0: pin control group to be used for this channel.
> +- pinctrl-names: must be "default".
> +- renesas,primary-bond: empty property indicating the channel acts as
> primary
> + among the bonded channels.
> +- port: child port node of a channel that defines the local and remote
> + endpoints. The remote endpoint is assumed to be a third party tuner
> + device endpoint.

You should refer to the OF graphs bindings here. How about the following to 
document the port node ?

- port: child port node corresponding to the data input, in accordance with 
the video interface bindings defined in
Documentation/devicetree/bindings/media/video-interfaces.txt. The port node 
must contain at least one endpoint.

> +Optional endpoint property:
> +---
> +- renesas,sync-active  : Indicates sync signal polarity, 0/1 for 

Re: [PATCH v4 3/9] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[Linus Walleij]

On Mon, Apr 10, 2017 at 8:12 PM, Rob Herring  wrote:
> On Wed, Apr 05, 2017 at 04:07:21PM +0200, Jacopo Mondi wrote:

>> +  The allowed generic formats for a pin multiplexing sub-node are the
>> +  following ones:
>> +
>> +  node-1 {
>> +  pinmux = , , ... ;
>> +  GENERIC_PINCONFIG;
>
> What's GENERIC_PINCONFIG? I see this in some other binding docs, but not
> used anywhere. If this is a boolean property then get rid of the all
> caps. If this is a define, then don't use complex defines that expand to
> dts source.

I guess it is a wildcard for everything under the heading in
"Generic pin configuration node content"
in Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

I'm all for documenting it properly.

It's kind of useful, but I don't know the recent ambtions about being
formal with DT bindings. The GPIO bindings are just over the top
with BNF notation in its formalism. Dunno what is best here :/

Yours,
Linus Walleij

RE: [PATCH v3 2/7] dt-bindings: media: Add MAX2175 binding description

[Ramesh Shanmugasundaram]

Hi Laurent,

Thanks for the review comments.

> 
> On Tuesday 07 Feb 2017 15:02:32 Ramesh Shanmugasundaram wrote:
> > Add device tree binding documentation for MAX2175 Rf to bits tuner
> > device.
> >
> > Signed-off-by: Ramesh Shanmugasundaram
> >  ---
> >  .../devicetree/bindings/media/i2c/max2175.txt  | 61
> +++
> >  .../devicetree/bindings/property-units.txt |  1 +
> >  2 files changed, 62 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/i2c/max2175.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt
> > b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file
> > mode
> > 100644
> > index 000..f591ab4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> > @@ -0,0 +1,61 @@
> > +Maxim Integrated MAX2175 RF to Bits tuner
> > +-
> > +
> > +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver
> > +with RF to Bits(r) front-end designed for software-defined radio
> solutions.
> > +
> > +Required properties:
> > +
> > +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner.
> > +- clocks: phandle to the fixed xtal clock.
> > +- clock-names: name of the fixed xtal clock.
> 
> I would mention that the name has to be "xtal". Maybe something like
> 
> - clock-names: name of the fixed xtal clock, shall be "xtal".

Agreed.

> 
> > +- port: child port node of a tuner that defines the local and remote
> > +  endpoints. The remote endpoint is assumed to be an SDR device
> > +  that is capable of receiving the digital samples from the tuner.
> 
> You should refer to the OF graphs bindings here. How about the following
> to document the port node ?
> 
> - port: child port node corresponding to the I2S output, in accordance
> with the video interface bindings defined in
> Documentation/devicetree/bindings/media/video-interfaces.txt. The port
> node must contain at least one endpoint.

Agreed.

> 
> > +Optional properties:
> > +
> > +- maxim,slave: phandle to the master tuner if it is a slave.
> This
> > +   is used to define two tuners in diversity mode
> > +   (1 master, 1 slave). By default each tuner is an
> > +   individual master.
> 
> It seems weird to me to name a property "slave" when it points to the
> master tuner. Shouldn't it be named "maxim,master" ?

Agreed.

> 
> > +- maxim,refout-load-pF: load capacitance value (in pF) on reference
> > +   output drive level. The possible load values are
> > +0 (default - refout disabled)
> > +   10
> > +   20
> > +   30
> > +   40
> > +   60
> > +   70
> > +- maxim,am-hiz   : empty property indicates AM Hi-Z filter path
> is
> > +   selected for AM antenna input. By default this
> > +   filter path is not used.
> 
> Isn't this something that should be selected at runtime through a control
> ? Or does the hardware design dictate whether the filter has to be used or
> must not be used ?

This is dictated by the h/w design and not selectable at run-time. 
I will update these changes in the next patchset.

Thanks,
Ramesh

Re: [PATCH v3 1/7] media: v4l2-ctrls: Reserve controls for MAX217X

[Laurent Pinchart]

Hi Ramesh,

Thank you for the patch.

On Tuesday 07 Feb 2017 15:02:31 Ramesh Shanmugasundaram wrote:
> Reserve controls for MAX217X RF to Bits tuner family. These hybrid
> radio receiver chips are highly programmable and hence reserving 32
> controls.
> 
> Signed-off-by: Ramesh Shanmugasundaram
> 

Acked-by: Laurent Pinchart 

> ---
>  include/uapi/linux/v4l2-controls.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h index 0d2e1e0..83b28b4 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -180,6 +180,11 @@ enum v4l2_colorfx {
>   * We reserve 16 controls for this driver. */
>  #define V4L2_CID_USER_TC358743_BASE  (V4L2_CID_USER_BASE + 0x1080)
> 
> +/* The base for the max217x driver controls.
> + * We reserve 32 controls for this driver
> + */
> +#define V4L2_CID_USER_MAX217X_BASE   (V4L2_CID_USER_BASE + 0x1090)
> +
>  /* MPEG-class control IDs */
>  /* The MPEG controls are applicable to all codec controls
>   * and the 'MPEG' part of the define is historical */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/9] pinctrl: generic: Add bi-directional and output-enable

[Linus Walleij]

On Wed, Apr 5, 2017 at 4:07 PM, Jacopo Mondi  wrote:

> Add bi-directional and output-enable pin configuration properties.
>
> bi-directional allows to specify when a pin shall operate in input and
> output mode at the same time. This is particularly useful in platforms
> where input and output buffers have to be manually enabled.
>
> output-enable is just syntactic sugar to specify that a pin shall
> operate in output mode, ignoring the provided argument.
> This pairs with input-enable pin configuration option.
>
> Signed-off-by: Jacopo Mondi 

Patch applied with Rob's ACK.

Yours,
Linus Walleij

Re: [PATCH v4 0/9] Renesas RZ/A1 pin and gpio controller

[Linus Walleij]

On Wed, Apr 5, 2017 at 4:07 PM, Jacopo Mondi  wrote:

> Hi Linus,
>this is 4th round of gpio/pincontroller for RZ/A1 devices.
>
> As you suggested in v3 review, I have now added what we called pinmux flags
> to the list of standard pinconf generic properties, and we're now using
> generic parsing routines to collect them and apply them when multiplexing
> pins.

I have merged patch 1/9 so you have the necessary infrastructure in place.

If Geert want to send a pull request based on my devel branch that is fine
(but a bit late) else the requirements are there for a merge in the early
v4.12 kernel cycle.

Yours,
Linus Walleij

Re: [PATCH v4 0/9] Renesas RZ/A1 pin and gpio controller

[Geert Uytterhoeven]

Hi Linus,

On Tue, Apr 11, 2017 at 11:05 AM, Linus Walleij
 wrote:
> On Wed, Apr 5, 2017 at 4:07 PM, Jacopo Mondi  
> wrote:
>> Hi Linus,
>>this is 4th round of gpio/pincontroller for RZ/A1 devices.
>>
>> As you suggested in v3 review, I have now added what we called pinmux flags
>> to the list of standard pinconf generic properties, and we're now using
>> generic parsing routines to collect them and apply them when multiplexing
>> pins.
>
> I have merged patch 1/9 so you have the necessary infrastructure in place.
>
> If Geert want to send a pull request based on my devel branch that is fine
> (but a bit late) else the requirements are there for a merge in the early
> v4.12 kernel cycle.

I agree it's a bit late. Will queue for v4.13.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 2/2] clk: cs2000: add AUX_OUT pin select support

[Kuninori Morimoto]

Hi Geert

Thank you for your feedback

> > +- auxoutsrc:   select AUX_OUT source from these.
> > +   refclk: Timing Reference Clock
> > +   clk_in: Frequency Reference Clock
> > +   pllclkout:  PLL Clock Output
> > +   push-pull:  PLL Lock/Unlock Indication
> > +   open-drain: PLL Lock/Unlock Indication
> 
> AUX_OUT is an output pin?
> Hence, isn't this software configuration instead of hardware description?
> 
> Selection of refclk vs. clk_in vs. pllclkout can be implemented as a mux clock
> driver with three parents.
> 
> PLL Lock/Unlock Indication and its pinctrl are something different.
> How to support that?

I think mux clock is nice idea.
I will use this idea.

Best regards
---
Kuninori Morimoto

RE: [PATCH v3 2/7] dt-bindings: media: Add MAX2175 binding description

[Ramesh Shanmugasundaram]

Hi Laurent,

> On Tuesday 11 Apr 2017 09:57:45 Ramesh Shanmugasundaram wrote:
> > > On Tuesday 07 Feb 2017 15:02:32 Ramesh Shanmugasundaram wrote:
> > >> Add device tree binding documentation for MAX2175 Rf to bits tuner
> > >> device.
> > >>
> > >> Signed-off-by: Ramesh Shanmugasundaram
> > >>  ---
> > >>
> > >>  .../devicetree/bindings/media/i2c/max2175.txt  | 61
> +++
> > >>  .../devicetree/bindings/property-units.txt |  1 +
> > >>  2 files changed, 62 insertions(+)
> > >>  create mode 100644
> > >>
> > >> Documentation/devicetree/bindings/media/i2c/max2175.txt
> > >>
> > >> diff --git
> > >> a/Documentation/devicetree/bindings/media/i2c/max2175.txt
> > >> b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file
> > >> mode 100644 index 000..f591ab4
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> 
> [snip]
> 
> > >> +- maxim,am-hiz: empty property indicates AM Hi-Z filter
> path
> > >> is
> > >> +selected for AM antenna input. By default this
> > >> +filter path is not used.
> > >
> > > Isn't this something that should be selected at runtime through a
> > > control ? Or does the hardware design dictate whether the filter has
> > > to be used or must not be used ?
> >
> > This is dictated by the h/w design and not selectable at run-time.
> > I will update these changes in the next patchset.
> 
> In that case I'm fine with a property, but could we name it in such a way
> that it describes the hardware instead of instructing the software on how
> to configure the device ? For instance (and this is a made-up example as I
> don't know exactly how this works), if the AM Hi-Z filter is required when
> dealing with AM frequencies and forbidden when dealing with other
> frequency bands, and
> *if* boards have to be designed specifically for one frequency band (AM,
> FM, VHF, L, ...) without any way to accept different bands, then you could
> instead use
> 
>   maxim,frequency-band = "AM";
> 
> and enable the filter accordingly in the driver. This would be in my
> opinion a better system hardware description.

I am not sure. The AM antenna input path has a default filter and AM Hi-Z 
filter. H/W dictates the path to be used for AM input only and this is fixed. 
The device can be configured to use different bands at runtime & not AM only. I 
could edit the description as below:

- maxim,am-hiz: empty property indicates AM Hi-Z filter path usage for 
AM antenna
input as dictated by hardware design. By default this 
filter path is not used.

Is it any better? Do you still think the property name should be changed please?

Thanks,
Ramesh

Re: [PATCH v3 2/7] dt-bindings: media: Add MAX2175 binding description

[Laurent Pinchart]

Hi Ramesh,

On Tuesday 11 Apr 2017 09:57:45 Ramesh Shanmugasundaram wrote:
> > On Tuesday 07 Feb 2017 15:02:32 Ramesh Shanmugasundaram wrote:
> >> Add device tree binding documentation for MAX2175 Rf to bits tuner
> >> device.
> >> 
> >> Signed-off-by: Ramesh Shanmugasundaram
> >>  ---
> >> 
> >>  .../devicetree/bindings/media/i2c/max2175.txt  | 61 +++
> >>  .../devicetree/bindings/property-units.txt |  1 +
> >>  2 files changed, 62 insertions(+)
> >>  create mode 100644
> >> 
> >> Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file
> >> mode 100644
> >> index 000..f591ab4
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt

[snip]

> >> +- maxim,am-hiz  : empty property indicates AM Hi-Z filter path
> >> is
> >> +  selected for AM antenna input. By default this
> >> +  filter path is not used.
> > 
> > Isn't this something that should be selected at runtime through a control
> > ? Or does the hardware design dictate whether the filter has to be used or
> > must not be used ?
> 
> This is dictated by the h/w design and not selectable at run-time.
> I will update these changes in the next patchset.

In that case I'm fine with a property, but could we name it in such a way that 
it describes the hardware instead of instructing the software on how to 
configure the device ? For instance (and this is a made-up example as I don't 
know exactly how this works), if the AM Hi-Z filter is required when dealing 
with AM frequencies and forbidden when dealing with other frequency bands, and 
*if* boards have to be designed specifically for one frequency band (AM, FM, 
VHF, L, ...) without any way to accept different bands, then you could instead 
use

maxim,frequency-band = "AM";

and enable the filter accordingly in the driver. This would be in my opinion a 
better system hardware description.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] clk: cs2000: add AUX_OUT pin select support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Tue, Apr 11, 2017 at 2:36 AM, Kuninori Morimoto
 wrote:
> From: Kuninori Morimoto 
>
> Signed-off-by: Kuninori Morimoto 
> Tested-by: Hiroyuki Yokoyama 
> ---
>  .../devicetree/bindings/clock/cs2000-cp.txt|  9 +++
>  drivers/clk/clk-cs2000-cp.c| 73 
> +-
>  2 files changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/cs2000-cp.txt 
> b/Documentation/devicetree/bindings/clock/cs2000-cp.txt
> index 54e6df0..4c2f9cb 100644
> --- a/Documentation/devicetree/bindings/clock/cs2000-cp.txt
> +++ b/Documentation/devicetree/bindings/clock/cs2000-cp.txt
> @@ -8,6 +8,15 @@ Required properties:
>  - clock-names: CLK_IN : clk_in, XTI/REF_CLK : ref_clk
>  - #clock-cells:must be <0>
>
> +Option properties:
> +
> +- auxoutsrc:   select AUX_OUT source from these.
> +   refclk: Timing Reference Clock
> +   clk_in: Frequency Reference Clock
> +   pllclkout:  PLL Clock Output
> +   push-pull:  PLL Lock/Unlock Indication
> +   open-drain: PLL Lock/Unlock Indication

AUX_OUT is an output pin?
Hence, isn't this software configuration instead of hardware description?

Selection of refclk vs. clk_in vs. pllclkout can be implemented as a mux clock
driver with three parents.

PLL Lock/Unlock Indication and its pinctrl are something different.
How to support that?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 2/7] dt-bindings: media: Add MAX2175 binding description

[Laurent Pinchart]

Hi Ramesh,

Thank you for the patch.

On Tuesday 07 Feb 2017 15:02:32 Ramesh Shanmugasundaram wrote:
> Add device tree binding documentation for MAX2175 Rf to bits tuner
> device.
> 
> Signed-off-by: Ramesh Shanmugasundaram
>  ---
>  .../devicetree/bindings/media/i2c/max2175.txt  | 61 +++
>  .../devicetree/bindings/property-units.txt |  1 +
>  2 files changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/max2175.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt
> b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file mode
> 100644
> index 000..f591ab4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> @@ -0,0 +1,61 @@
> +Maxim Integrated MAX2175 RF to Bits tuner
> +-
> +
> +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver with
> +RF to Bits® front-end designed for software-defined radio solutions.
> +
> +Required properties:
> +
> +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner.
> +- clocks: phandle to the fixed xtal clock.
> +- clock-names: name of the fixed xtal clock.

I would mention that the name has to be "xtal". Maybe something like

- clock-names: name of the fixed xtal clock, shall be "xtal".

> +- port: child port node of a tuner that defines the local and remote
> +  endpoints. The remote endpoint is assumed to be an SDR device
> +  that is capable of receiving the digital samples from the tuner.

You should refer to the OF graphs bindings here. How about the following to 
document the port node ?

- port: child port node corresponding to the I2S output, in accordance with 
the video interface bindings defined in
Documentation/devicetree/bindings/media/video-interfaces.txt. The port node 
must contain at least one endpoint.

> +Optional properties:
> +
> +- maxim,slave  : phandle to the master tuner if it is a slave. 
This
> + is used to define two tuners in diversity mode
> + (1 master, 1 slave). By default each tuner is an
> + individual master.

It seems weird to me to name a property "slave" when it points to the master 
tuner. Shouldn't it be named "maxim,master" ?

> +- maxim,refout-load-pF: load capacitance value (in pF) on reference
> + output drive level. The possible load values are
> +  0 (default - refout disabled)
> + 10
> + 20
> + 30
> + 40
> + 60
> + 70
> +- maxim,am-hiz : empty property indicates AM Hi-Z filter path 
is
> + selected for AM antenna input. By default this
> + filter path is not used.

Isn't this something that should be selected at runtime through a control ? Or 
does the hardware design dictate whether the filter has to be used or must not 
be used ?

> +Example:
> +
> +
> +Board specific DTS file
> +
> +/* Fixed XTAL clock node */
> +maxim_xtal: clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <36864000>;
> +};
> +
> +/* A tuner device instance under i2c bus */
> +max2175_0: tuner@60 {
> + compatible = "maxim,max2175";
> + reg = <0x60>;
> + clocks = <_xtal>;
> + clock-names = "xtal";
> + maxim,refout-load-pF = <10>;
> +
> + port {
> + max2175_0_ep: endpoint {
> + remote-endpoint = <_rx_device>;
> + };
> + };
> +
> +};
> diff --git a/Documentation/devicetree/bindings/property-units.txt
> b/Documentation/devicetree/bindings/property-units.txt index
> 12278d7..f1f1c22 100644
> --- a/Documentation/devicetree/bindings/property-units.txt
> +++ b/Documentation/devicetree/bindings/property-units.txt
> @@ -28,6 +28,7 @@ Electricity
>  -ohms: Ohms
>  -micro-ohms  : micro Ohms
>  -microvolt   : micro volts
> +-pF  : pico farads
> 
>  Temperature
>  

-- 
Regards,

Laurent Pinchart