Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-07-20 Thread Luciano Coelho
On Thu, 2013-07-18 at 01:58 +0200, Laurent Pinchart wrote:
> Hi Luciano,

Hi Laurent,

> On Monday 01 July 2013 15:39:30 Luciano Coelho wrote:
> > The only thing I can come up with is to make a small clock driver (maybe
> > even inside the WiLink module itself) that registers a new type of
> > clock, "ti,wilink-clock" or something.  But this would really be
> > overkill, wouldn't it?
> > 
> > Any other ideas?
> 
> One possibility would be to just call clk_get_rate() on the clock from the 
> WiLink driver, which would return the fixed frequency specified in DT, and 
> configure the WiLink hardware accordingly. This might be a bit hackish though.

The problem is not get the rate itself, the problem is knowing whether
the clock is XTAL or not.  The WiLink chip uses the clock in a slightly
different way if it is XTAL, due to some stabilization time constraints.

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-07-20 Thread Luciano Coelho
On Thu, 2013-07-18 at 01:58 +0200, Laurent Pinchart wrote:
 Hi Luciano,

Hi Laurent,

 On Monday 01 July 2013 15:39:30 Luciano Coelho wrote:
  The only thing I can come up with is to make a small clock driver (maybe
  even inside the WiLink module itself) that registers a new type of
  clock, ti,wilink-clock or something.  But this would really be
  overkill, wouldn't it?
  
  Any other ideas?
 
 One possibility would be to just call clk_get_rate() on the clock from the 
 WiLink driver, which would return the fixed frequency specified in DT, and 
 configure the WiLink hardware accordingly. This might be a bit hackish though.

The problem is not get the rate itself, the problem is knowing whether
the clock is XTAL or not.  The WiLink chip uses the clock in a slightly
different way if it is XTAL, due to some stabilization time constraints.

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-07-17 Thread Laurent Pinchart
Hi Luciano,

On Monday 01 July 2013 15:39:30 Luciano Coelho wrote:
> On Fri, 2013-06-28 at 16:21 +0300, Luciano Coelho wrote:
> > On Fri, 2013-06-28 at 15:18 +0300, Felipe Balbi wrote:
> > > On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
> > > > On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
> > > > > On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
> > > > > > On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> > > > > > > (fixed Mike's address)
> > > > > > > 
> > > > > > > On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > > > > > > > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho 
wrote:
> > > > > > > > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > > > > > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho 
wrote:
> > > > > > > > > > > +Optional properties:
> > > > > > > > > > > +
> > > > > > > > > > > +
> > > > > > > > > > > +- refclock: the internal WLAN reference clock frequency
> > > > > > > > > > > (required for
> > > > > > > > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be
> > > > > > > > > > > one of the
> > > > > > > > > > > +  following:
> > > > > > > > > > > + 0 = 19.2 MHz
> > > > > > > > > > > + 1 = 26.0 MHz
> > > > > > > > > > > + 2 = 38.4 MHz
> > > > > > > > > > > + 3 = 52.0 MHz
> > > > > > > > > > > + 4 = 38.4 MHz, XTAL
> > > > > > > > > > > + 5 = 26.0 MHz, XTAL
> > > > > > > > > > > +
> > > > > > > > > > > +- tcxoclock: the internal WLAN TCXO clock frequency
> > > > > > > > > > > (required for
> > > > > > > > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be
> > > > > > > > > > > one of the
> > > > > > > > > > > +  following:
> > > > > > > > > > > + 0 = 19.200 MHz
> > > > > > > > > > > + 1 = 26.000 MHz
> > > > > > > > > > > + 2 = 38.400 MHz
> > > > > > > > > > > + 3 = 52.000 MHz
> > > > > > > > > > > + 4 = 16.368 MHz
> > > > > > > > > > > + 5 = 32.736 MHz
> > > > > > > > > > > + 6 = 16.800 MHz
> > > > > > > > > > > + 7 = 33.600 MHz
> > > > > > > > > > 
> > > > > > > > > > This looks suspiciously like what we have the common clock
> > > > > > > > > > bindings for:
> > > > > > > > > > 
> > > > > > > > > > refclk {
> > > > > > > > > > 
> > > > > > > > > > compatible = "fixed-clock";
> > > > > > > > > > #clock-cells = <0>;
> > > > > > > > > > clock-frequency = <1920>;
> > > > > > > > > > 
> > > > > > > > > > }
> > > > > > > > > > 
> > > > > > > > > > wilink {
> > > > > > > > > > 
> > > > > > > > > > compatible = "ti,wilink7";
> > > > > > > > > > interrupt-parent = <_interrupt_controller>;
> > > > > > > > > > interrupts = <0 1 1>;
> > > > > > > > > > clocks = <>, <>;
> > > > > > > > > > clock-names = "refclk", "txoclk";
> > > > > > > > > > 
> > > > > > > > > > };
> > > > > > > > > > 
> > > > > > > > > > Could you not use them?
> > > > > > > > > 
> > > > > > > > > Hmmm... this actually does look good.  But these are
> > > > > > > > > internal clocks in the modules, they cannot be accessed from
> > > > > > > > > outside.  Does it make sense to register them with the clock
> > > > > > > > > framework?
> > > > > > > > 
> > > > > > > > Given we already have a common way of describing clocks, I
> > > > > > > > think it makes sense to use it -- people already understand
> > > > > > > > the common bindings, and it's less code to add add to the
> > > > > > > > kernel. I don't think the fact these clocks are internal
> > > > > > > > should prevent us from describing them as we would an external
> > > > > > > > clock.
> > > > > > > 
> > > > > > > Yes, I agree with you.  Thanks for the suggestion! I think it
> > > > > > > will look much better.  And now that I dug a bit more into the
> > > > > > > code, I can see that there are only structs being populated, so
> > > > > > > there shouldn't be any other side-effects.
> > > > > > 
> > > > > > Hmmm, one thing that escaped me.  Besides the frequency, I also
> > > > > > need a boolean that tells if the clock is XTAL or not.  I can't
> > > > > > figure out how to pass this if I use the generic clock framework. 
> > > > > > Any suggestions?
> > > > > 
> > > > > Could you use clock-output-names for that ?
> > > > > 
> > > > > XTAL clock:
> > > > > 
> > > > > refclk {
> > > > > 
> > > > >   compatible = "fixed-clock";
> > > > >   #clock cells = <0>;
> > > > >   clock-frequency = <1920>;
> > > > >   clock-output-names = "xtal";
> > > > > 
> > > > > };
> > > > > 
> > > > > non-XTAL clock:
> > > > > 
> > > > > refclk {
> > > > > 
> > > > >   compatible = "fixed-clock";
> > > > >   #clock cells = <0>;
> > > > >   clock-frequency = <1920>;
> > > > >   clock-output-names = "osc"; /* any better name ? */
> > > > > 
> > > > > };
> > > > 
> > > > This starts looking a bit hacky.  Using the output name as a flag is
> > > > not very pretty.
> > > > 
> > > > I think it would be better to have a separate flag for it in the wlan
> > > > node.  Like an optional 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-07-17 Thread Laurent Pinchart
Hi Luciano,

On Monday 01 July 2013 15:39:30 Luciano Coelho wrote:
 On Fri, 2013-06-28 at 16:21 +0300, Luciano Coelho wrote:
  On Fri, 2013-06-28 at 15:18 +0300, Felipe Balbi wrote:
   On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
 On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
  On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
   (fixed Mike's address)
   
   On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho 
wrote:
 On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
  On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho 
wrote:
   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency
   (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be
   one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
   +
   +- tcxoclock: the internal WLAN TCXO clock frequency
   (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be
   one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  This looks suspiciously like what we have the common clock
  bindings for:
  
  refclk {
  
  compatible = fixed-clock;
  #clock-cells = 0;
  clock-frequency = 1920;
  
  }
  
  wilink {
  
  compatible = ti,wilink7;
  interrupt-parent = some_interrupt_controller;
  interrupts = 0 1 1;
  clocks = refclk, refclk;
  clock-names = refclk, txoclk;
  
  };
  
  Could you not use them?
 
 Hmmm... this actually does look good.  But these are
 internal clocks in the modules, they cannot be accessed from
 outside.  Does it make sense to register them with the clock
 framework?

Given we already have a common way of describing clocks, I
think it makes sense to use it -- people already understand
the common bindings, and it's less code to add add to the
kernel. I don't think the fact these clocks are internal
should prevent us from describing them as we would an external
clock.
   
   Yes, I agree with you.  Thanks for the suggestion! I think it
   will look much better.  And now that I dug a bit more into the
   code, I can see that there are only structs being populated, so
   there shouldn't be any other side-effects.
  
  Hmmm, one thing that escaped me.  Besides the frequency, I also
  need a boolean that tells if the clock is XTAL or not.  I can't
  figure out how to pass this if I use the generic clock framework. 
  Any suggestions?
 
 Could you use clock-output-names for that ?
 
 XTAL clock:
 
 refclk {
 
   compatible = fixed-clock;
   #clock cells = 0;
   clock-frequency = 1920;
   clock-output-names = xtal;
 
 };
 
 non-XTAL clock:
 
 refclk {
 
   compatible = fixed-clock;
   #clock cells = 0;
   clock-frequency = 1920;
   clock-output-names = osc; /* any better name ? */
 
 };

This starts looking a bit hacky.  Using the output name as a flag is
not very pretty.

I think it would be better to have a separate flag for it in the wlan
node.  Like an optional refclock-xtal boolean or something.  The
downside of this is that we would be adding information about the
clock details in the wilink node. :(

OTOH, we could add a flag to the generic clock binding? A new optional
boolean that tells whether the clock is XTAL or not:

refclk {

compatible = fixed-clock;
#clock cells = 0;
clock-frequency = 1920;
clock-xtal;

};

Do you think that would make sense?
   
   sure, that looks alright to me. Surely there are other devices out there
   who want to know if the clock comes from a crystal or not ?!?
  
  Mike, what do you think about this idea? If it sounds okay to you, I can
  cook up a patch adding this flag.
 
 Hmmm... I started implementing this whole thing, but using these clocks
 as fixed-clocks is not so straightforward.  The problem is that I
 would need to register my driver as a 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-07-01 Thread Luciano Coelho
On Fri, 2013-06-28 at 16:21 +0300, Luciano Coelho wrote:
> On Fri, 2013-06-28 at 15:18 +0300, Felipe Balbi wrote:
> > On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
> > > On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
> > > > On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
> > > > > On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> > > > > > (fixed Mike's address)
> > > > > > 
> > > > > > On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > > > > > > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > > > > > > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > > > > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho 
> > > > > > > > > wrote:
> > > > > > > > > > +Optional properties:
> > > > > > > > > > +
> > > > > > > > > > +
> > > > > > > > > > +- refclock: the internal WLAN reference clock frequency 
> > > > > > > > > > (required for
> > > > > > > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one 
> > > > > > > > > > of the
> > > > > > > > > > +  following:
> > > > > > > > > > +   0 = 19.2 MHz
> > > > > > > > > > +   1 = 26.0 MHz
> > > > > > > > > > +   2 = 38.4 MHz
> > > > > > > > > > +   3 = 52.0 MHz
> > > > > > > > > > +   4 = 38.4 MHz, XTAL
> > > > > > > > > > +   5 = 26.0 MHz, XTAL
> > > > > > > > > > +
> > > > > > > > > > +- tcxoclock: the internal WLAN TCXO clock frequency 
> > > > > > > > > > (required for
> > > > > > > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one 
> > > > > > > > > > of the
> > > > > > > > > > +  following:
> > > > > > > > > > +   0 = 19.200 MHz
> > > > > > > > > > +   1 = 26.000 MHz
> > > > > > > > > > +   2 = 38.400 MHz
> > > > > > > > > > +   3 = 52.000 MHz
> > > > > > > > > > +   4 = 16.368 MHz
> > > > > > > > > > +   5 = 32.736 MHz
> > > > > > > > > > +   6 = 16.800 MHz
> > > > > > > > > > +   7 = 33.600 MHz
> > > > > > > > > 
> > > > > > > > > This looks suspiciously like what we have the common clock 
> > > > > > > > > bindings for:
> > > > > > > > > 
> > > > > > > > > refclk {
> > > > > > > > >   compatible = "fixed-clock";
> > > > > > > > >   #clock-cells = <0>;
> > > > > > > > >   clock-frequency = <1920>;
> > > > > > > > > }
> > > > > > > > > 
> > > > > > > > > wilink {
> > > > > > > > >   compatible = "ti,wilink7";
> > > > > > > > >   interrupt-parent = <_interrupt_controller>;
> > > > > > > > >   interrupts = <0 1 1>;
> > > > > > > > >   clocks = <>, <>;
> > > > > > > > >   clock-names = "refclk", "txoclk";
> > > > > > > > > };
> > > > > > > > > 
> > > > > > > > > Could you not use them?
> > > > > > > > 
> > > > > > > > Hmmm... this actually does look good.  But these are internal 
> > > > > > > > clocks in
> > > > > > > > the modules, they cannot be accessed from outside.  Does it 
> > > > > > > > make sense
> > > > > > > > to register them with the clock framework?
> > > > > > > 
> > > > > > > Given we already have a common way of describing clocks, I think 
> > > > > > > it
> > > > > > > makes sense to use it -- people already understand the common 
> > > > > > > bindings,
> > > > > > > and it's less code to add add to the kernel. I don't think the 
> > > > > > > fact
> > > > > > > these clocks are internal should prevent us from describing them 
> > > > > > > as we
> > > > > > > would an external clock.
> > > > > > 
> > > > > > Yes, I agree with you.  Thanks for the suggestion! I think it will 
> > > > > > look
> > > > > > much better.  And now that I dug a bit more into the code, I can see
> > > > > > that there are only structs being populated, so there shouldn't be 
> > > > > > any
> > > > > > other side-effects.
> > > > > 
> > > > > Hmmm, one thing that escaped me.  Besides the frequency, I also need a
> > > > > boolean that tells if the clock is XTAL or not.  I can't figure out 
> > > > > how
> > > > > to pass this if I use the generic clock framework.  Any suggestions?
> > > > 
> > > > Could you use clock-output-names for that ?
> > > > 
> > > > XTAL clock:
> > > > 
> > > > refclk {
> > > > compatible = "fixed-clock";
> > > > #clock cells = <0>;
> > > > clock-frequency = <1920>;
> > > > clock-output-names = "xtal";
> > > > };
> > > > 
> > > > non-XTAL clock:
> > > > 
> > > > refclk {
> > > > compatible = "fixed-clock";
> > > > #clock cells = <0>;
> > > > clock-frequency = <1920>;
> > > > clock-output-names = "osc"; /* any better name ? */
> > > > };
> > > 
> > > This starts looking a bit hacky.  Using the output name as a flag is not
> > > very pretty.
> > > 
> > > I think it would be better to have a separate flag for it in the wlan
> > > node.  Like an optional "refclock-xtal" boolean or something.  The
> > > downside of this is that we would be adding information about the clock
> > > details in the wilink node. :(
> > > 
> > > OTOH, we could add a flag to the generic clock binding? A new 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-07-01 Thread Luciano Coelho
On Fri, 2013-06-28 at 16:21 +0300, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 15:18 +0300, Felipe Balbi wrote:
  On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
   On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
  (fixed Mike's address)
  
  On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
   On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
 On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho 
 wrote:
  +Optional properties:
  +
  +
  +- refclock: the internal WLAN reference clock frequency 
  (required for
  +  WiLink6 and WiLink7; not used for WiLink8).  Must be one 
  of the
  +  following:
  +   0 = 19.2 MHz
  +   1 = 26.0 MHz
  +   2 = 38.4 MHz
  +   3 = 52.0 MHz
  +   4 = 38.4 MHz, XTAL
  +   5 = 26.0 MHz, XTAL
  +
  +- tcxoclock: the internal WLAN TCXO clock frequency 
  (required for
  +  WiLink7 not used for WiLink6 and WiLink8).  Must be one 
  of the
  +  following:
  +   0 = 19.200 MHz
  +   1 = 26.000 MHz
  +   2 = 38.400 MHz
  +   3 = 52.000 MHz
  +   4 = 16.368 MHz
  +   5 = 32.736 MHz
  +   6 = 16.800 MHz
  +   7 = 33.600 MHz
 
 This looks suspiciously like what we have the common clock 
 bindings for:
 
 refclk {
   compatible = fixed-clock;
   #clock-cells = 0;
   clock-frequency = 1920;
 }
 
 wilink {
   compatible = ti,wilink7;
   interrupt-parent = some_interrupt_controller;
   interrupts = 0 1 1;
   clocks = refclk, refclk;
   clock-names = refclk, txoclk;
 };
 
 Could you not use them?

Hmmm... this actually does look good.  But these are internal 
clocks in
the modules, they cannot be accessed from outside.  Does it 
make sense
to register them with the clock framework?
   
   Given we already have a common way of describing clocks, I think 
   it
   makes sense to use it -- people already understand the common 
   bindings,
   and it's less code to add add to the kernel. I don't think the 
   fact
   these clocks are internal should prevent us from describing them 
   as we
   would an external clock.
  
  Yes, I agree with you.  Thanks for the suggestion! I think it will 
  look
  much better.  And now that I dug a bit more into the code, I can see
  that there are only structs being populated, so there shouldn't be 
  any
  other side-effects.
 
 Hmmm, one thing that escaped me.  Besides the frequency, I also need a
 boolean that tells if the clock is XTAL or not.  I can't figure out 
 how
 to pass this if I use the generic clock framework.  Any suggestions?

Could you use clock-output-names for that ?

XTAL clock:

refclk {
compatible = fixed-clock;
#clock cells = 0;
clock-frequency = 1920;
clock-output-names = xtal;
};

non-XTAL clock:

refclk {
compatible = fixed-clock;
#clock cells = 0;
clock-frequency = 1920;
clock-output-names = osc; /* any better name ? */
};
   
   This starts looking a bit hacky.  Using the output name as a flag is not
   very pretty.
   
   I think it would be better to have a separate flag for it in the wlan
   node.  Like an optional refclock-xtal boolean or something.  The
   downside of this is that we would be adding information about the clock
   details in the wilink node. :(
   
   OTOH, we could add a flag to the generic clock binding? A new optional
   boolean that tells whether the clock is XTAL or not:
   
   refclk {
 compatible = fixed-clock;
 #clock cells = 0;
 clock-frequency = 1920;
 clock-xtal;
   };
   
   Do you think that would make sense?
  
  sure, that looks alright to me. Surely there are other devices out there
  who want to know if the clock comes from a crystal or not ?!?
 
 Mike, what do you think about this idea? If it sounds okay to you, I can
 cook up a patch adding this flag.

Hmmm... I started implementing this whole thing, but using these clocks
as fixed-clocks is not so straightforward.  The problem is that I
would need to register my driver as a clock provider and add the OF
match for fixed-clock.

If I do that, all the other fixed-clock nodes would be passed to my
driver too, which is wrong.  Or, the platform should 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 15:18 +0300, Felipe Balbi wrote:
> On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
> > On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
> > > On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
> > > > On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> > > > > (fixed Mike's address)
> > > > > 
> > > > > On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > > > > > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > > > > > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > > > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > > > > > > > +Optional properties:
> > > > > > > > > +
> > > > > > > > > +
> > > > > > > > > +- refclock: the internal WLAN reference clock frequency 
> > > > > > > > > (required for
> > > > > > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one 
> > > > > > > > > of the
> > > > > > > > > +  following:
> > > > > > > > > + 0 = 19.2 MHz
> > > > > > > > > + 1 = 26.0 MHz
> > > > > > > > > + 2 = 38.4 MHz
> > > > > > > > > + 3 = 52.0 MHz
> > > > > > > > > + 4 = 38.4 MHz, XTAL
> > > > > > > > > + 5 = 26.0 MHz, XTAL
> > > > > > > > > +
> > > > > > > > > +- tcxoclock: the internal WLAN TCXO clock frequency 
> > > > > > > > > (required for
> > > > > > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of 
> > > > > > > > > the
> > > > > > > > > +  following:
> > > > > > > > > + 0 = 19.200 MHz
> > > > > > > > > + 1 = 26.000 MHz
> > > > > > > > > + 2 = 38.400 MHz
> > > > > > > > > + 3 = 52.000 MHz
> > > > > > > > > + 4 = 16.368 MHz
> > > > > > > > > + 5 = 32.736 MHz
> > > > > > > > > + 6 = 16.800 MHz
> > > > > > > > > + 7 = 33.600 MHz
> > > > > > > > 
> > > > > > > > This looks suspiciously like what we have the common clock 
> > > > > > > > bindings for:
> > > > > > > > 
> > > > > > > > refclk {
> > > > > > > > compatible = "fixed-clock";
> > > > > > > > #clock-cells = <0>;
> > > > > > > > clock-frequency = <1920>;
> > > > > > > > }
> > > > > > > > 
> > > > > > > > wilink {
> > > > > > > > compatible = "ti,wilink7";
> > > > > > > > interrupt-parent = <_interrupt_controller>;
> > > > > > > > interrupts = <0 1 1>;
> > > > > > > > clocks = <>, <>;
> > > > > > > > clock-names = "refclk", "txoclk";
> > > > > > > > };
> > > > > > > > 
> > > > > > > > Could you not use them?
> > > > > > > 
> > > > > > > Hmmm... this actually does look good.  But these are internal 
> > > > > > > clocks in
> > > > > > > the modules, they cannot be accessed from outside.  Does it make 
> > > > > > > sense
> > > > > > > to register them with the clock framework?
> > > > > > 
> > > > > > Given we already have a common way of describing clocks, I think it
> > > > > > makes sense to use it -- people already understand the common 
> > > > > > bindings,
> > > > > > and it's less code to add add to the kernel. I don't think the fact
> > > > > > these clocks are internal should prevent us from describing them as 
> > > > > > we
> > > > > > would an external clock.
> > > > > 
> > > > > Yes, I agree with you.  Thanks for the suggestion! I think it will 
> > > > > look
> > > > > much better.  And now that I dug a bit more into the code, I can see
> > > > > that there are only structs being populated, so there shouldn't be any
> > > > > other side-effects.
> > > > 
> > > > Hmmm, one thing that escaped me.  Besides the frequency, I also need a
> > > > boolean that tells if the clock is XTAL or not.  I can't figure out how
> > > > to pass this if I use the generic clock framework.  Any suggestions?
> > > 
> > > Could you use clock-output-names for that ?
> > > 
> > > XTAL clock:
> > > 
> > > refclk {
> > >   compatible = "fixed-clock";
> > >   #clock cells = <0>;
> > >   clock-frequency = <1920>;
> > >   clock-output-names = "xtal";
> > > };
> > > 
> > > non-XTAL clock:
> > > 
> > > refclk {
> > >   compatible = "fixed-clock";
> > >   #clock cells = <0>;
> > >   clock-frequency = <1920>;
> > >   clock-output-names = "osc"; /* any better name ? */
> > > };
> > 
> > This starts looking a bit hacky.  Using the output name as a flag is not
> > very pretty.
> > 
> > I think it would be better to have a separate flag for it in the wlan
> > node.  Like an optional "refclock-xtal" boolean or something.  The
> > downside of this is that we would be adding information about the clock
> > details in the wilink node. :(
> > 
> > OTOH, we could add a flag to the generic clock binding? A new optional
> > boolean that tells whether the clock is XTAL or not:
> > 
> > refclk {
> > compatible = "fixed-clock";
> > #clock cells = <0>;
> > clock-frequency = <1920>;
> > clock-xtal;
> > };
> > 
> > Do you think that would make sense?
> 
> sure, that looks alright to me. Surely there are other devices out there
> who want to know if the 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Felipe Balbi
On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
> On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
> > On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
> > > On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> > > > (fixed Mike's address)
> > > > 
> > > > On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > > > > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > > > > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > > > > > > +Optional properties:
> > > > > > > > +
> > > > > > > > +
> > > > > > > > +- refclock: the internal WLAN reference clock frequency 
> > > > > > > > (required for
> > > > > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of 
> > > > > > > > the
> > > > > > > > +  following:
> > > > > > > > +   0 = 19.2 MHz
> > > > > > > > +   1 = 26.0 MHz
> > > > > > > > +   2 = 38.4 MHz
> > > > > > > > +   3 = 52.0 MHz
> > > > > > > > +   4 = 38.4 MHz, XTAL
> > > > > > > > +   5 = 26.0 MHz, XTAL
> > > > > > > > +
> > > > > > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required 
> > > > > > > > for
> > > > > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of 
> > > > > > > > the
> > > > > > > > +  following:
> > > > > > > > +   0 = 19.200 MHz
> > > > > > > > +   1 = 26.000 MHz
> > > > > > > > +   2 = 38.400 MHz
> > > > > > > > +   3 = 52.000 MHz
> > > > > > > > +   4 = 16.368 MHz
> > > > > > > > +   5 = 32.736 MHz
> > > > > > > > +   6 = 16.800 MHz
> > > > > > > > +   7 = 33.600 MHz
> > > > > > > 
> > > > > > > This looks suspiciously like what we have the common clock 
> > > > > > > bindings for:
> > > > > > > 
> > > > > > > refclk {
> > > > > > >   compatible = "fixed-clock";
> > > > > > >   #clock-cells = <0>;
> > > > > > >   clock-frequency = <1920>;
> > > > > > > }
> > > > > > > 
> > > > > > > wilink {
> > > > > > >   compatible = "ti,wilink7";
> > > > > > >   interrupt-parent = <_interrupt_controller>;
> > > > > > >   interrupts = <0 1 1>;
> > > > > > >   clocks = <>, <>;
> > > > > > >   clock-names = "refclk", "txoclk";
> > > > > > > };
> > > > > > > 
> > > > > > > Could you not use them?
> > > > > > 
> > > > > > Hmmm... this actually does look good.  But these are internal 
> > > > > > clocks in
> > > > > > the modules, they cannot be accessed from outside.  Does it make 
> > > > > > sense
> > > > > > to register them with the clock framework?
> > > > > 
> > > > > Given we already have a common way of describing clocks, I think it
> > > > > makes sense to use it -- people already understand the common 
> > > > > bindings,
> > > > > and it's less code to add add to the kernel. I don't think the fact
> > > > > these clocks are internal should prevent us from describing them as we
> > > > > would an external clock.
> > > > 
> > > > Yes, I agree with you.  Thanks for the suggestion! I think it will look
> > > > much better.  And now that I dug a bit more into the code, I can see
> > > > that there are only structs being populated, so there shouldn't be any
> > > > other side-effects.
> > > 
> > > Hmmm, one thing that escaped me.  Besides the frequency, I also need a
> > > boolean that tells if the clock is XTAL or not.  I can't figure out how
> > > to pass this if I use the generic clock framework.  Any suggestions?
> > 
> > Could you use clock-output-names for that ?
> > 
> > XTAL clock:
> > 
> > refclk {
> > compatible = "fixed-clock";
> > #clock cells = <0>;
> > clock-frequency = <1920>;
> > clock-output-names = "xtal";
> > };
> > 
> > non-XTAL clock:
> > 
> > refclk {
> > compatible = "fixed-clock";
> > #clock cells = <0>;
> > clock-frequency = <1920>;
> > clock-output-names = "osc"; /* any better name ? */
> > };
> 
> This starts looking a bit hacky.  Using the output name as a flag is not
> very pretty.
> 
> I think it would be better to have a separate flag for it in the wlan
> node.  Like an optional "refclock-xtal" boolean or something.  The
> downside of this is that we would be adding information about the clock
> details in the wilink node. :(
> 
> OTOH, we could add a flag to the generic clock binding? A new optional
> boolean that tells whether the clock is XTAL or not:
> 
> refclk {
>   compatible = "fixed-clock";
>   #clock cells = <0>;
>   clock-frequency = <1920>;
>   clock-xtal;
> };
> 
> Do you think that would make sense?

sure, that looks alright to me. Surely there are other devices out there
who want to know if the clock comes from a crystal or not ?!?

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
> On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
> > On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> > > (fixed Mike's address)
> > > 
> > > On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > > > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > > > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > > > > > +Optional properties:
> > > > > > > +
> > > > > > > +
> > > > > > > +- refclock: the internal WLAN reference clock frequency 
> > > > > > > (required for
> > > > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > > > > > +  following:
> > > > > > > + 0 = 19.2 MHz
> > > > > > > + 1 = 26.0 MHz
> > > > > > > + 2 = 38.4 MHz
> > > > > > > + 3 = 52.0 MHz
> > > > > > > + 4 = 38.4 MHz, XTAL
> > > > > > > + 5 = 26.0 MHz, XTAL
> > > > > > > +
> > > > > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > > > > > +  following:
> > > > > > > + 0 = 19.200 MHz
> > > > > > > + 1 = 26.000 MHz
> > > > > > > + 2 = 38.400 MHz
> > > > > > > + 3 = 52.000 MHz
> > > > > > > + 4 = 16.368 MHz
> > > > > > > + 5 = 32.736 MHz
> > > > > > > + 6 = 16.800 MHz
> > > > > > > + 7 = 33.600 MHz
> > > > > > 
> > > > > > This looks suspiciously like what we have the common clock bindings 
> > > > > > for:
> > > > > > 
> > > > > > refclk {
> > > > > > compatible = "fixed-clock";
> > > > > > #clock-cells = <0>;
> > > > > > clock-frequency = <1920>;
> > > > > > }
> > > > > > 
> > > > > > wilink {
> > > > > > compatible = "ti,wilink7";
> > > > > > interrupt-parent = <_interrupt_controller>;
> > > > > > interrupts = <0 1 1>;
> > > > > > clocks = <>, <>;
> > > > > > clock-names = "refclk", "txoclk";
> > > > > > };
> > > > > > 
> > > > > > Could you not use them?
> > > > > 
> > > > > Hmmm... this actually does look good.  But these are internal clocks 
> > > > > in
> > > > > the modules, they cannot be accessed from outside.  Does it make sense
> > > > > to register them with the clock framework?
> > > > 
> > > > Given we already have a common way of describing clocks, I think it
> > > > makes sense to use it -- people already understand the common bindings,
> > > > and it's less code to add add to the kernel. I don't think the fact
> > > > these clocks are internal should prevent us from describing them as we
> > > > would an external clock.
> > > 
> > > Yes, I agree with you.  Thanks for the suggestion! I think it will look
> > > much better.  And now that I dug a bit more into the code, I can see
> > > that there are only structs being populated, so there shouldn't be any
> > > other side-effects.
> > 
> > Hmmm, one thing that escaped me.  Besides the frequency, I also need a
> > boolean that tells if the clock is XTAL or not.  I can't figure out how
> > to pass this if I use the generic clock framework.  Any suggestions?
> 
> Could you use clock-output-names for that ?
> 
> XTAL clock:
> 
> refclk {
>   compatible = "fixed-clock";
>   #clock cells = <0>;
>   clock-frequency = <1920>;
>   clock-output-names = "xtal";
> };
> 
> non-XTAL clock:
> 
> refclk {
>   compatible = "fixed-clock";
>   #clock cells = <0>;
>   clock-frequency = <1920>;
>   clock-output-names = "osc"; /* any better name ? */
> };

This starts looking a bit hacky.  Using the output name as a flag is not
very pretty.

I think it would be better to have a separate flag for it in the wlan
node.  Like an optional "refclock-xtal" boolean or something.  The
downside of this is that we would be adding information about the clock
details in the wilink node. :(

OTOH, we could add a flag to the generic clock binding? A new optional
boolean that tells whether the clock is XTAL or not:

refclk {
compatible = "fixed-clock";
#clock cells = <0>;
clock-frequency = <1920>;
clock-xtal;
};

Do you think that would make sense?

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Felipe Balbi
On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
> On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> > (fixed Mike's address)
> > 
> > On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > > > > +Optional properties:
> > > > > > +
> > > > > > +
> > > > > > +- refclock: the internal WLAN reference clock frequency (required 
> > > > > > for
> > > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > > > > +  following:
> > > > > > +   0 = 19.2 MHz
> > > > > > +   1 = 26.0 MHz
> > > > > > +   2 = 38.4 MHz
> > > > > > +   3 = 52.0 MHz
> > > > > > +   4 = 38.4 MHz, XTAL
> > > > > > +   5 = 26.0 MHz, XTAL
> > > > > > +
> > > > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > > > > +  following:
> > > > > > +   0 = 19.200 MHz
> > > > > > +   1 = 26.000 MHz
> > > > > > +   2 = 38.400 MHz
> > > > > > +   3 = 52.000 MHz
> > > > > > +   4 = 16.368 MHz
> > > > > > +   5 = 32.736 MHz
> > > > > > +   6 = 16.800 MHz
> > > > > > +   7 = 33.600 MHz
> > > > > 
> > > > > This looks suspiciously like what we have the common clock bindings 
> > > > > for:
> > > > > 
> > > > > refclk {
> > > > >   compatible = "fixed-clock";
> > > > >   #clock-cells = <0>;
> > > > >   clock-frequency = <1920>;
> > > > > }
> > > > > 
> > > > > wilink {
> > > > >   compatible = "ti,wilink7";
> > > > >   interrupt-parent = <_interrupt_controller>;
> > > > >   interrupts = <0 1 1>;
> > > > >   clocks = <>, <>;
> > > > >   clock-names = "refclk", "txoclk";
> > > > > };
> > > > > 
> > > > > Could you not use them?
> > > > 
> > > > Hmmm... this actually does look good.  But these are internal clocks in
> > > > the modules, they cannot be accessed from outside.  Does it make sense
> > > > to register them with the clock framework?
> > > 
> > > Given we already have a common way of describing clocks, I think it
> > > makes sense to use it -- people already understand the common bindings,
> > > and it's less code to add add to the kernel. I don't think the fact
> > > these clocks are internal should prevent us from describing them as we
> > > would an external clock.
> > 
> > Yes, I agree with you.  Thanks for the suggestion! I think it will look
> > much better.  And now that I dug a bit more into the code, I can see
> > that there are only structs being populated, so there shouldn't be any
> > other side-effects.
> 
> Hmmm, one thing that escaped me.  Besides the frequency, I also need a
> boolean that tells if the clock is XTAL or not.  I can't figure out how
> to pass this if I use the generic clock framework.  Any suggestions?

Could you use clock-output-names for that ?

XTAL clock:

refclk {
compatible = "fixed-clock";
#clock cells = <0>;
clock-frequency = <1920>;
clock-output-names = "xtal";
};

non-XTAL clock:

refclk {
compatible = "fixed-clock";
#clock cells = <0>;
clock-frequency = <1920>;
clock-output-names = "osc"; /* any better name ? */
};

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
> (fixed Mike's address)
> 
> On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > > > +Optional properties:
> > > > > +
> > > > > +
> > > > > +- refclock: the internal WLAN reference clock frequency (required for
> > > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > > > +  following:
> > > > > + 0 = 19.2 MHz
> > > > > + 1 = 26.0 MHz
> > > > > + 2 = 38.4 MHz
> > > > > + 3 = 52.0 MHz
> > > > > + 4 = 38.4 MHz, XTAL
> > > > > + 5 = 26.0 MHz, XTAL
> > > > > +
> > > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > > > +  following:
> > > > > + 0 = 19.200 MHz
> > > > > + 1 = 26.000 MHz
> > > > > + 2 = 38.400 MHz
> > > > > + 3 = 52.000 MHz
> > > > > + 4 = 16.368 MHz
> > > > > + 5 = 32.736 MHz
> > > > > + 6 = 16.800 MHz
> > > > > + 7 = 33.600 MHz
> > > > 
> > > > This looks suspiciously like what we have the common clock bindings for:
> > > > 
> > > > refclk {
> > > > compatible = "fixed-clock";
> > > > #clock-cells = <0>;
> > > > clock-frequency = <1920>;
> > > > }
> > > > 
> > > > wilink {
> > > > compatible = "ti,wilink7";
> > > > interrupt-parent = <_interrupt_controller>;
> > > > interrupts = <0 1 1>;
> > > > clocks = <>, <>;
> > > > clock-names = "refclk", "txoclk";
> > > > };
> > > > 
> > > > Could you not use them?
> > > 
> > > Hmmm... this actually does look good.  But these are internal clocks in
> > > the modules, they cannot be accessed from outside.  Does it make sense
> > > to register them with the clock framework?
> > 
> > Given we already have a common way of describing clocks, I think it
> > makes sense to use it -- people already understand the common bindings,
> > and it's less code to add add to the kernel. I don't think the fact
> > these clocks are internal should prevent us from describing them as we
> > would an external clock.
> 
> Yes, I agree with you.  Thanks for the suggestion! I think it will look
> much better.  And now that I dug a bit more into the code, I can see
> that there are only structs being populated, so there shouldn't be any
> other side-effects.

Hmmm, one thing that escaped me.  Besides the frequency, I also need a
boolean that tells if the clock is XTAL or not.  I can't figure out how
to pass this if I use the generic clock framework.  Any suggestions?

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
[resending again with the doubly corrected address for Mike Turquette,
apologies for the spam]

On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > +Optional properties:
> > > +
> > > +
> > > +- refclock: the internal WLAN reference clock frequency (required for
> > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.2 MHz
> > > + 1 = 26.0 MHz
> > > + 2 = 38.4 MHz
> > > + 3 = 52.0 MHz
> > > + 4 = 38.4 MHz, XTAL
> > > + 5 = 26.0 MHz, XTAL
> > > +
> > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.200 MHz
> > > + 1 = 26.000 MHz
> > > + 2 = 38.400 MHz
> > > + 3 = 52.000 MHz
> > > + 4 = 16.368 MHz
> > > + 5 = 32.736 MHz
> > > + 6 = 16.800 MHz
> > > + 7 = 33.600 MHz
> > 
> > This looks suspiciously like what we have the common clock bindings for:
> > 
> > refclk {
> > compatible = "fixed-clock";
> > #clock-cells = <0>;
> > clock-frequency = <1920>;
> > }
> > 
> > wilink {
> > compatible = "ti,wilink7";
> > interrupt-parent = <_interrupt_controller>;
> > interrupts = <0 1 1>;
> > clocks = <>, <>;
> > clock-names = "refclk", "txoclk";
> > };
> > 
> > Could you not use them?
> 
> Hmmm... this actually does look good.  But these are internal clocks in
> the modules, they cannot be accessed from outside.  Does it make sense
> to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Thanks,
Mark.
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
[resending with the correct address for Mike Turquette]

On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > +Optional properties:
> > > +
> > > +
> > > +- refclock: the internal WLAN reference clock frequency (required for
> > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.2 MHz
> > > + 1 = 26.0 MHz
> > > + 2 = 38.4 MHz
> > > + 3 = 52.0 MHz
> > > + 4 = 38.4 MHz, XTAL
> > > + 5 = 26.0 MHz, XTAL
> > > +
> > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.200 MHz
> > > + 1 = 26.000 MHz
> > > + 2 = 38.400 MHz
> > > + 3 = 52.000 MHz
> > > + 4 = 16.368 MHz
> > > + 5 = 32.736 MHz
> > > + 6 = 16.800 MHz
> > > + 7 = 33.600 MHz
> > 
> > This looks suspiciously like what we have the common clock bindings for:
> > 
> > refclk {
> > compatible = "fixed-clock";
> > #clock-cells = <0>;
> > clock-frequency = <1920>;
> > }
> > 
> > wilink {
> > compatible = "ti,wilink7";
> > interrupt-parent = <_interrupt_controller>;
> > interrupts = <0 1 1>;
> > clocks = <>, <>;
> > clock-names = "refclk", "txoclk";
> > };
> > 
> > Could you not use them?
> 
> Hmmm... this actually does look good.  But these are internal clocks in
> the modules, they cannot be accessed from outside.  Does it make sense
> to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Thanks,
Mark.
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
(fixed Mike's address)

On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
> On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > > +Optional properties:
> > > > +
> > > > +
> > > > +- refclock: the internal WLAN reference clock frequency (required for
> > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > > +  following:
> > > > +   0 = 19.2 MHz
> > > > +   1 = 26.0 MHz
> > > > +   2 = 38.4 MHz
> > > > +   3 = 52.0 MHz
> > > > +   4 = 38.4 MHz, XTAL
> > > > +   5 = 26.0 MHz, XTAL
> > > > +
> > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > > +  following:
> > > > +   0 = 19.200 MHz
> > > > +   1 = 26.000 MHz
> > > > +   2 = 38.400 MHz
> > > > +   3 = 52.000 MHz
> > > > +   4 = 16.368 MHz
> > > > +   5 = 32.736 MHz
> > > > +   6 = 16.800 MHz
> > > > +   7 = 33.600 MHz
> > > 
> > > This looks suspiciously like what we have the common clock bindings for:
> > > 
> > > refclk {
> > >   compatible = "fixed-clock";
> > >   #clock-cells = <0>;
> > >   clock-frequency = <1920>;
> > > }
> > > 
> > > wilink {
> > >   compatible = "ti,wilink7";
> > >   interrupt-parent = <_interrupt_controller>;
> > >   interrupts = <0 1 1>;
> > >   clocks = <>, <>;
> > >   clock-names = "refclk", "txoclk";
> > > };
> > > 
> > > Could you not use them?
> > 
> > Hmmm... this actually does look good.  But these are internal clocks in
> > the modules, they cannot be accessed from outside.  Does it make sense
> > to register them with the clock framework?
> 
> Given we already have a common way of describing clocks, I think it
> makes sense to use it -- people already understand the common bindings,
> and it's less code to add add to the kernel. I don't think the fact
> these clocks are internal should prevent us from describing them as we
> would an external clock.

Yes, I agree with you.  Thanks for the suggestion! I think it will look
much better.  And now that I dug a bit more into the code, I can see
that there are only structs being populated, so there shouldn't be any
other side-effects.


> Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Experts' opinions are appreciated. :)

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
> On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > > +Optional properties:
> > > +
> > > +
> > > +- refclock: the internal WLAN reference clock frequency (required for
> > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.2 MHz
> > > + 1 = 26.0 MHz
> > > + 2 = 38.4 MHz
> > > + 3 = 52.0 MHz
> > > + 4 = 38.4 MHz, XTAL
> > > + 5 = 26.0 MHz, XTAL
> > > +
> > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.200 MHz
> > > + 1 = 26.000 MHz
> > > + 2 = 38.400 MHz
> > > + 3 = 52.000 MHz
> > > + 4 = 16.368 MHz
> > > + 5 = 32.736 MHz
> > > + 6 = 16.800 MHz
> > > + 7 = 33.600 MHz
> > 
> > This looks suspiciously like what we have the common clock bindings for:
> > 
> > refclk {
> > compatible = "fixed-clock";
> > #clock-cells = <0>;
> > clock-frequency = <1920>;
> > }
> > 
> > wilink {
> > compatible = "ti,wilink7";
> > interrupt-parent = <_interrupt_controller>;
> > interrupts = <0 1 1>;
> > clocks = <>, <>;
> > clock-names = "refclk", "txoclk";
> > };
> > 
> > Could you not use them?
> 
> Hmmm... this actually does look good.  But these are internal clocks in
> the modules, they cannot be accessed from outside.  Does it make sense
> to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
> On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> > +Optional properties:
> > +
> > +
> > +- refclock: the internal WLAN reference clock frequency (required for
> > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.2 MHz
> > +   1 = 26.0 MHz
> > +   2 = 38.4 MHz
> > +   3 = 52.0 MHz
> > +   4 = 38.4 MHz, XTAL
> > +   5 = 26.0 MHz, XTAL
> > +
> > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.200 MHz
> > +   1 = 26.000 MHz
> > +   2 = 38.400 MHz
> > +   3 = 52.000 MHz
> > +   4 = 16.368 MHz
> > +   5 = 32.736 MHz
> > +   6 = 16.800 MHz
> > +   7 = 33.600 MHz
> 
> This looks suspiciously like what we have the common clock bindings for:
> 
> refclk {
>   compatible = "fixed-clock";
>   #clock-cells = <0>;
>   clock-frequency = <1920>;
> }
> 
> wilink {
>   compatible = "ti,wilink7";
>   interrupt-parent = <_interrupt_controller>;
>   interrupts = <0 1 1>;
>   clocks = <>, <>;
>   clock-names = "refclk", "txoclk";
> };
> 
> Could you not use them?

Hmmm... this actually does look good.  But these are internal clocks in
the modules, they cannot be accessed from outside.  Does it make sense
to register them with the clock framework?

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
> Add device tree bindings documentation for the TI WiLink modules.
> Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> modules is supported.
> 
> Signed-off-by: Luciano Coelho 
> ---
> 
> I created a new directory under net to contain wireless bindings 
> documentation.
> 
> The actual implementation in the driver will follow separately.
> 
>  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
> 
>  1 file changed, 46 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
> b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> new file mode 100644
> index 000..d8e8bfbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> @@ -0,0 +1,46 @@
> +TI WiLink Wireless Modules Device Tree Bindings
> +===
> +
> +The WiLink modules provide wireless connectivity, such as WLAN,
> +Bluetooth, FM and NFC.
> +
> +There are several different modules available, which can be grouped by
> +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
> +currently supported with device tree.
> +
> +Currently, only the WLAN portion of the modules is supported with
> +device tree.
> +
> +Required properties:
> +
> +
> +- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
> +- interrupt-parent: the interrupt controller
> +- interrupts: out-of-band WLAN interrupt
> + See the interrupt controller's bindings documentation for
> + detailed definition.
> +
> +Optional properties:
> +
> +
> +- refclock: the internal WLAN reference clock frequency (required for
> +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> +  following:
> + 0 = 19.2 MHz
> + 1 = 26.0 MHz
> + 2 = 38.4 MHz
> + 3 = 52.0 MHz
> + 4 = 38.4 MHz, XTAL
> + 5 = 26.0 MHz, XTAL
> +
> +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> +  following:
> + 0 = 19.200 MHz
> + 1 = 26.000 MHz
> + 2 = 38.400 MHz
> + 3 = 52.000 MHz
> + 4 = 16.368 MHz
> + 5 = 32.736 MHz
> + 6 = 16.800 MHz
> + 7 = 33.600 MHz

This looks suspiciously like what we have the common clock bindings for:

refclk {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <1920>;
}

wilink {
compatible = "ti,wilink7";
interrupt-parent = <_interrupt_controller>;
interrupts = <0 1 1>;
clocks = <>, <>;
clock-names = "refclk", "txoclk";
};

Could you not use them?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
 Add device tree bindings documentation for the TI WiLink modules.
 Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
 modules is supported.
 
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---
 
 I created a new directory under net to contain wireless bindings 
 documentation.
 
 The actual implementation in the driver will follow separately.
 
  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
 
  1 file changed, 46 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 
 diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
 b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 new file mode 100644
 index 000..d8e8bfbb
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 @@ -0,0 +1,46 @@
 +TI WiLink Wireless Modules Device Tree Bindings
 +===
 +
 +The WiLink modules provide wireless connectivity, such as WLAN,
 +Bluetooth, FM and NFC.
 +
 +There are several different modules available, which can be grouped by
 +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
 +currently supported with device tree.
 +
 +Currently, only the WLAN portion of the modules is supported with
 +device tree.
 +
 +Required properties:
 +
 +
 +- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
 +- interrupt-parent: the interrupt controller
 +- interrupts: out-of-band WLAN interrupt
 + See the interrupt controller's bindings documentation for
 + detailed definition.
 +
 +Optional properties:
 +
 +
 +- refclock: the internal WLAN reference clock frequency (required for
 +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
 +  following:
 + 0 = 19.2 MHz
 + 1 = 26.0 MHz
 + 2 = 38.4 MHz
 + 3 = 52.0 MHz
 + 4 = 38.4 MHz, XTAL
 + 5 = 26.0 MHz, XTAL
 +
 +- tcxoclock: the internal WLAN TCXO clock frequency (required for
 +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
 +  following:
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz

This looks suspiciously like what we have the common clock bindings for:

refclk {
compatible = fixed-clock;
#clock-cells = 0;
clock-frequency = 1920;
}

wilink {
compatible = ti,wilink7;
interrupt-parent = some_interrupt_controller;
interrupts = 0 1 1;
clocks = refclk, refclk;
clock-names = refclk, txoclk;
};

Could you not use them?

Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
 On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
  +Optional properties:
  +
  +
  +- refclock: the internal WLAN reference clock frequency (required for
  +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
  +  following:
  +   0 = 19.2 MHz
  +   1 = 26.0 MHz
  +   2 = 38.4 MHz
  +   3 = 52.0 MHz
  +   4 = 38.4 MHz, XTAL
  +   5 = 26.0 MHz, XTAL
  +
  +- tcxoclock: the internal WLAN TCXO clock frequency (required for
  +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
  +  following:
  +   0 = 19.200 MHz
  +   1 = 26.000 MHz
  +   2 = 38.400 MHz
  +   3 = 52.000 MHz
  +   4 = 16.368 MHz
  +   5 = 32.736 MHz
  +   6 = 16.800 MHz
  +   7 = 33.600 MHz
 
 This looks suspiciously like what we have the common clock bindings for:
 
 refclk {
   compatible = fixed-clock;
   #clock-cells = 0;
   clock-frequency = 1920;
 }
 
 wilink {
   compatible = ti,wilink7;
   interrupt-parent = some_interrupt_controller;
   interrupts = 0 1 1;
   clocks = refclk, refclk;
   clock-names = refclk, txoclk;
 };
 
 Could you not use them?

Hmmm... this actually does look good.  But these are internal clocks in
the modules, they cannot be accessed from outside.  Does it make sense
to register them with the clock framework?

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
  On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
   +
   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  This looks suspiciously like what we have the common clock bindings for:
  
  refclk {
  compatible = fixed-clock;
  #clock-cells = 0;
  clock-frequency = 1920;
  }
  
  wilink {
  compatible = ti,wilink7;
  interrupt-parent = some_interrupt_controller;
  interrupts = 0 1 1;
  clocks = refclk, refclk;
  clock-names = refclk, txoclk;
  };
  
  Could you not use them?
 
 Hmmm... this actually does look good.  But these are internal clocks in
 the modules, they cannot be accessed from outside.  Does it make sense
 to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
(fixed Mike's address)

On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
 On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
  On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
   On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
   
   This looks suspiciously like what we have the common clock bindings for:
   
   refclk {
 compatible = fixed-clock;
 #clock-cells = 0;
 clock-frequency = 1920;
   }
   
   wilink {
 compatible = ti,wilink7;
 interrupt-parent = some_interrupt_controller;
 interrupts = 0 1 1;
 clocks = refclk, refclk;
 clock-names = refclk, txoclk;
   };
   
   Could you not use them?
  
  Hmmm... this actually does look good.  But these are internal clocks in
  the modules, they cannot be accessed from outside.  Does it make sense
  to register them with the clock framework?
 
 Given we already have a common way of describing clocks, I think it
 makes sense to use it -- people already understand the common bindings,
 and it's less code to add add to the kernel. I don't think the fact
 these clocks are internal should prevent us from describing them as we
 would an external clock.

Yes, I agree with you.  Thanks for the suggestion! I think it will look
much better.  And now that I dug a bit more into the code, I can see
that there are only structs being populated, so there shouldn't be any
other side-effects.


 Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Experts' opinions are appreciated. :)

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
[resending with the correct address for Mike Turquette]

On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
  On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
   +
   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  This looks suspiciously like what we have the common clock bindings for:
  
  refclk {
  compatible = fixed-clock;
  #clock-cells = 0;
  clock-frequency = 1920;
  }
  
  wilink {
  compatible = ti,wilink7;
  interrupt-parent = some_interrupt_controller;
  interrupts = 0 1 1;
  clocks = refclk, refclk;
  clock-names = refclk, txoclk;
  };
  
  Could you not use them?
 
 Hmmm... this actually does look good.  But these are internal clocks in
 the modules, they cannot be accessed from outside.  Does it make sense
 to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Thanks,
Mark.
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Mark Rutland
[resending again with the doubly corrected address for Mike Turquette,
apologies for the spam]

On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
  On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
   +
   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  This looks suspiciously like what we have the common clock bindings for:
  
  refclk {
  compatible = fixed-clock;
  #clock-cells = 0;
  clock-frequency = 1920;
  }
  
  wilink {
  compatible = ti,wilink7;
  interrupt-parent = some_interrupt_controller;
  interrupts = 0 1 1;
  clocks = refclk, refclk;
  clock-names = refclk, txoclk;
  };
  
  Could you not use them?
 
 Hmmm... this actually does look good.  But these are internal clocks in
 the modules, they cannot be accessed from outside.  Does it make sense
 to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Perhaps Mike Turquette [Cc'd] has an opinion on the matter. 

Thanks,
Mark.
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
 (fixed Mike's address)
 
 On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
  On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
   On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
 +Optional properties:
 +
 +
 +- refclock: the internal WLAN reference clock frequency (required for
 +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
 +  following:
 + 0 = 19.2 MHz
 + 1 = 26.0 MHz
 + 2 = 38.4 MHz
 + 3 = 52.0 MHz
 + 4 = 38.4 MHz, XTAL
 + 5 = 26.0 MHz, XTAL
 +
 +- tcxoclock: the internal WLAN TCXO clock frequency (required for
 +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
 +  following:
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz

This looks suspiciously like what we have the common clock bindings for:

refclk {
compatible = fixed-clock;
#clock-cells = 0;
clock-frequency = 1920;
}

wilink {
compatible = ti,wilink7;
interrupt-parent = some_interrupt_controller;
interrupts = 0 1 1;
clocks = refclk, refclk;
clock-names = refclk, txoclk;
};

Could you not use them?
   
   Hmmm... this actually does look good.  But these are internal clocks in
   the modules, they cannot be accessed from outside.  Does it make sense
   to register them with the clock framework?
  
  Given we already have a common way of describing clocks, I think it
  makes sense to use it -- people already understand the common bindings,
  and it's less code to add add to the kernel. I don't think the fact
  these clocks are internal should prevent us from describing them as we
  would an external clock.
 
 Yes, I agree with you.  Thanks for the suggestion! I think it will look
 much better.  And now that I dug a bit more into the code, I can see
 that there are only structs being populated, so there shouldn't be any
 other side-effects.

Hmmm, one thing that escaped me.  Besides the frequency, I also need a
boolean that tells if the clock is XTAL or not.  I can't figure out how
to pass this if I use the generic clock framework.  Any suggestions?

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Felipe Balbi
On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
  (fixed Mike's address)
  
  On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
   On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
 On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
  +Optional properties:
  +
  +
  +- refclock: the internal WLAN reference clock frequency (required 
  for
  +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
  +  following:
  +   0 = 19.2 MHz
  +   1 = 26.0 MHz
  +   2 = 38.4 MHz
  +   3 = 52.0 MHz
  +   4 = 38.4 MHz, XTAL
  +   5 = 26.0 MHz, XTAL
  +
  +- tcxoclock: the internal WLAN TCXO clock frequency (required for
  +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
  +  following:
  +   0 = 19.200 MHz
  +   1 = 26.000 MHz
  +   2 = 38.400 MHz
  +   3 = 52.000 MHz
  +   4 = 16.368 MHz
  +   5 = 32.736 MHz
  +   6 = 16.800 MHz
  +   7 = 33.600 MHz
 
 This looks suspiciously like what we have the common clock bindings 
 for:
 
 refclk {
   compatible = fixed-clock;
   #clock-cells = 0;
   clock-frequency = 1920;
 }
 
 wilink {
   compatible = ti,wilink7;
   interrupt-parent = some_interrupt_controller;
   interrupts = 0 1 1;
   clocks = refclk, refclk;
   clock-names = refclk, txoclk;
 };
 
 Could you not use them?

Hmmm... this actually does look good.  But these are internal clocks in
the modules, they cannot be accessed from outside.  Does it make sense
to register them with the clock framework?
   
   Given we already have a common way of describing clocks, I think it
   makes sense to use it -- people already understand the common bindings,
   and it's less code to add add to the kernel. I don't think the fact
   these clocks are internal should prevent us from describing them as we
   would an external clock.
  
  Yes, I agree with you.  Thanks for the suggestion! I think it will look
  much better.  And now that I dug a bit more into the code, I can see
  that there are only structs being populated, so there shouldn't be any
  other side-effects.
 
 Hmmm, one thing that escaped me.  Besides the frequency, I also need a
 boolean that tells if the clock is XTAL or not.  I can't figure out how
 to pass this if I use the generic clock framework.  Any suggestions?

Could you use clock-output-names for that ?

XTAL clock:

refclk {
compatible = fixed-clock;
#clock cells = 0;
clock-frequency = 1920;
clock-output-names = xtal;
};

non-XTAL clock:

refclk {
compatible = fixed-clock;
#clock cells = 0;
clock-frequency = 1920;
clock-output-names = osc; /* any better name ? */
};

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
 On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
  On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
   (fixed Mike's address)
   
   On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
  On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency 
   (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
   +
   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  This looks suspiciously like what we have the common clock bindings 
  for:
  
  refclk {
  compatible = fixed-clock;
  #clock-cells = 0;
  clock-frequency = 1920;
  }
  
  wilink {
  compatible = ti,wilink7;
  interrupt-parent = some_interrupt_controller;
  interrupts = 0 1 1;
  clocks = refclk, refclk;
  clock-names = refclk, txoclk;
  };
  
  Could you not use them?
 
 Hmmm... this actually does look good.  But these are internal clocks 
 in
 the modules, they cannot be accessed from outside.  Does it make sense
 to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.
   
   Yes, I agree with you.  Thanks for the suggestion! I think it will look
   much better.  And now that I dug a bit more into the code, I can see
   that there are only structs being populated, so there shouldn't be any
   other side-effects.
  
  Hmmm, one thing that escaped me.  Besides the frequency, I also need a
  boolean that tells if the clock is XTAL or not.  I can't figure out how
  to pass this if I use the generic clock framework.  Any suggestions?
 
 Could you use clock-output-names for that ?
 
 XTAL clock:
 
 refclk {
   compatible = fixed-clock;
   #clock cells = 0;
   clock-frequency = 1920;
   clock-output-names = xtal;
 };
 
 non-XTAL clock:
 
 refclk {
   compatible = fixed-clock;
   #clock cells = 0;
   clock-frequency = 1920;
   clock-output-names = osc; /* any better name ? */
 };

This starts looking a bit hacky.  Using the output name as a flag is not
very pretty.

I think it would be better to have a separate flag for it in the wlan
node.  Like an optional refclock-xtal boolean or something.  The
downside of this is that we would be adding information about the clock
details in the wilink node. :(

OTOH, we could add a flag to the generic clock binding? A new optional
boolean that tells whether the clock is XTAL or not:

refclk {
compatible = fixed-clock;
#clock cells = 0;
clock-frequency = 1920;
clock-xtal;
};

Do you think that would make sense?

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Felipe Balbi
On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
 On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
  On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
   On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
(fixed Mike's address)

On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
 On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
  On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
   On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency 
(required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of 
the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required 
for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of 
the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
   
   This looks suspiciously like what we have the common clock 
   bindings for:
   
   refclk {
 compatible = fixed-clock;
 #clock-cells = 0;
 clock-frequency = 1920;
   }
   
   wilink {
 compatible = ti,wilink7;
 interrupt-parent = some_interrupt_controller;
 interrupts = 0 1 1;
 clocks = refclk, refclk;
 clock-names = refclk, txoclk;
   };
   
   Could you not use them?
  
  Hmmm... this actually does look good.  But these are internal 
  clocks in
  the modules, they cannot be accessed from outside.  Does it make 
  sense
  to register them with the clock framework?
 
 Given we already have a common way of describing clocks, I think it
 makes sense to use it -- people already understand the common 
 bindings,
 and it's less code to add add to the kernel. I don't think the fact
 these clocks are internal should prevent us from describing them as we
 would an external clock.

Yes, I agree with you.  Thanks for the suggestion! I think it will look
much better.  And now that I dug a bit more into the code, I can see
that there are only structs being populated, so there shouldn't be any
other side-effects.
   
   Hmmm, one thing that escaped me.  Besides the frequency, I also need a
   boolean that tells if the clock is XTAL or not.  I can't figure out how
   to pass this if I use the generic clock framework.  Any suggestions?
  
  Could you use clock-output-names for that ?
  
  XTAL clock:
  
  refclk {
  compatible = fixed-clock;
  #clock cells = 0;
  clock-frequency = 1920;
  clock-output-names = xtal;
  };
  
  non-XTAL clock:
  
  refclk {
  compatible = fixed-clock;
  #clock cells = 0;
  clock-frequency = 1920;
  clock-output-names = osc; /* any better name ? */
  };
 
 This starts looking a bit hacky.  Using the output name as a flag is not
 very pretty.
 
 I think it would be better to have a separate flag for it in the wlan
 node.  Like an optional refclock-xtal boolean or something.  The
 downside of this is that we would be adding information about the clock
 details in the wilink node. :(
 
 OTOH, we could add a flag to the generic clock binding? A new optional
 boolean that tells whether the clock is XTAL or not:
 
 refclk {
   compatible = fixed-clock;
   #clock cells = 0;
   clock-frequency = 1920;
   clock-xtal;
 };
 
 Do you think that would make sense?

sure, that looks alright to me. Surely there are other devices out there
who want to know if the clock comes from a crystal or not ?!?

cheers

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-28 Thread Luciano Coelho
On Fri, 2013-06-28 at 15:18 +0300, Felipe Balbi wrote:
 On Fri, Jun 28, 2013 at 03:13:52PM +0300, Luciano Coelho wrote:
  On Fri, 2013-06-28 at 14:41 +0300, Felipe Balbi wrote:
   On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:
On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:
 (fixed Mike's address)
 
 On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:
  On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:
   On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:
On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:
 +Optional properties:
 +
 +
 +- refclock: the internal WLAN reference clock frequency 
 (required for
 +  WiLink6 and WiLink7; not used for WiLink8).  Must be one 
 of the
 +  following:
 + 0 = 19.2 MHz
 + 1 = 26.0 MHz
 + 2 = 38.4 MHz
 + 3 = 52.0 MHz
 + 4 = 38.4 MHz, XTAL
 + 5 = 26.0 MHz, XTAL
 +
 +- tcxoclock: the internal WLAN TCXO clock frequency 
 (required for
 +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of 
 the
 +  following:
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz

This looks suspiciously like what we have the common clock 
bindings for:

refclk {
compatible = fixed-clock;
#clock-cells = 0;
clock-frequency = 1920;
}

wilink {
compatible = ti,wilink7;
interrupt-parent = some_interrupt_controller;
interrupts = 0 1 1;
clocks = refclk, refclk;
clock-names = refclk, txoclk;
};

Could you not use them?
   
   Hmmm... this actually does look good.  But these are internal 
   clocks in
   the modules, they cannot be accessed from outside.  Does it make 
   sense
   to register them with the clock framework?
  
  Given we already have a common way of describing clocks, I think it
  makes sense to use it -- people already understand the common 
  bindings,
  and it's less code to add add to the kernel. I don't think the fact
  these clocks are internal should prevent us from describing them as 
  we
  would an external clock.
 
 Yes, I agree with you.  Thanks for the suggestion! I think it will 
 look
 much better.  And now that I dug a bit more into the code, I can see
 that there are only structs being populated, so there shouldn't be any
 other side-effects.

Hmmm, one thing that escaped me.  Besides the frequency, I also need a
boolean that tells if the clock is XTAL or not.  I can't figure out how
to pass this if I use the generic clock framework.  Any suggestions?
   
   Could you use clock-output-names for that ?
   
   XTAL clock:
   
   refclk {
 compatible = fixed-clock;
 #clock cells = 0;
 clock-frequency = 1920;
 clock-output-names = xtal;
   };
   
   non-XTAL clock:
   
   refclk {
 compatible = fixed-clock;
 #clock cells = 0;
 clock-frequency = 1920;
 clock-output-names = osc; /* any better name ? */
   };
  
  This starts looking a bit hacky.  Using the output name as a flag is not
  very pretty.
  
  I think it would be better to have a separate flag for it in the wlan
  node.  Like an optional refclock-xtal boolean or something.  The
  downside of this is that we would be adding information about the clock
  details in the wilink node. :(
  
  OTOH, we could add a flag to the generic clock binding? A new optional
  boolean that tells whether the clock is XTAL or not:
  
  refclk {
  compatible = fixed-clock;
  #clock cells = 0;
  clock-frequency = 1920;
  clock-xtal;
  };
  
  Do you think that would make sense?
 
 sure, that looks alright to me. Surely there are other devices out there
 who want to know if the clock comes from a crystal or not ?!?

Mike, what do you think about this idea? If it sounds okay to you, I can
cook up a patch adding this flag.

--
Cheers,
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon

On 06/27/2013 02:46 PM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 14:12 -0500, Nishanth Menon wrote:

[...]

Indexes to another entity is always a maintenance burden in the longer
run and needs judicious control. If it is possible to avoid it by
selecting the right parameters, I am a hard advocate for the same.


I tend to agree.  But you need a balance.  In theory you're right.  But
I think if you take the real world example, it is over-engineering.

Anyway, if you *really* think this needs to be changed, I think we're in
a deadlock here and I'd like to hear other people's opinions.  I don't
mind making the change, but I'm still not convinced it is worth it,
since it just adds complexity.

And hey, this is too much bikeshedding for such a small detail.

Lol :)

Alrite, if no one else is complaining, I am not going to block it either.

--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 14:12 -0500, Nishanth Menon wrote:
> On 06/27/2013 01:51 PM, Luciano Coelho wrote:
> > On Thu, 2013-06-27 at 08:39 -0500, Nishanth Menon wrote:
> >> On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho  wrote:
> >>> On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
>  On 06/27/2013 08:19 AM, Luciano Coelho wrote:
> > On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
> >> On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:
> >>> For the actual DTS files, I could add a wilink.dtsi with enumerations
> >>> for these values so they could be used in the node definitions.  But 
> >>> I'm
> >>> not sure it's going to be that valuable in the end.
> >> The  way GPIO HIGH was defined might help to provide guidance I think 
> >> :)
> >
> > Where? As far as I can see, the GPIO flags are defined in a bitmap.
> 
>  include/dt-bindings/gpio/gpio.h
> >>>
> >>> Thanks! I don't see these macros used anywhere, though.
> >> umm... I'd think you have'nt looked deep enough / lists :)
> >
> > If you mean mailing lists, you're right, I didn't.  I just did a git
> > grep for the macros and didn't find any users.
> git grep "GPIO_ACTIVE_[HIGH|LOW]" arch/arm/boot/dts/|wc -l
> 344
> on next-20130626. anyways, besides the point.
> 
> 
> >>> This seems to be a completely different thing.  This is the header that
> >>> contains the helper functions to get GPIO-related device tree nodes,
> >>> isn't it?
> >> That is true, but it also contains the flag for level which needs to
> >> be in sync with the gpio.h dts header.
>  just a hint. not saying frequencies were defined in header. for systems
>  that define frequencies - example cpufreq OPPs, clock node usage, we do
>  not use indexing to frequency, instead, that is the responsibility of
>  driver to convert frequency back to required index.
>  git grep frequency Documentation/devicetree/bindings gives you how the
>  precedence looks like.
> 
>  Personally, if given a choice, I'd go with actual frequencies rather
>  than indexes.
> >>>
> >>> If I do that, I need to add also a separate flag to define whether the
> >>> XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
> >>> crystal; and 38.4MHz and 38.4MHz crystal...
> >> Yes, you would have to. at the same time, it is easy for module maker
> >> to provide dtsi corresponding to exact h/w representation on his
> >> module using wilink chip without being worried about weird index value
> >> whose meaning is hidden in binding
> >
> > The module makers need to know about the bindings and read the document
> > before they specify the node in DTS.  I think for clarity, a comment in
> > the DTS file stating the actual frequency is good enough.  Simpler and
> > more efficient (in terms of DT binary size and module code size) than
> > adding the actual frequencies.
> >
> >
> >> On the flip side, It also allows driver to reject frequencies /
> >> configurations that are not supported by the corresponding chip.
> >
> > It's actually much easier to reject frequencies that are not valid with
> > the enumeration.  "if (refclock > 5) { bail_out(); }".  If I need to
> > check every frequency, I need to add an array of valid frequencies and
> > so on.  Waste of code, IMHO.
> >
> >
> >> As I said, just my 2 cents. Personally, I'd like dts files to be as
> >> readable as c files without having to dig through bindings document.
> >
> > As I said before, for readability, adding a comment with the frequency
> > is good enough.  This is what I did for PandaES's DTS (not sent out
> > yet):
> >
> > wlan {
> >  compatible = "ti,wilink6";
> >  interrupt-parent = <>;
> >  interrupts = <21 0x4>; /* gpio line 53, high level triggered */
> >  refclock = <2>;/* 38.4 MHz */
> >  };
> >
> > Looks more readable to me than:
> >
> > wlan {
> >  compatible = "ti,wilink6";
> >  interrupt-parent = <>;
> >  interrupts = <21 0x4>; /* gpio line 53, high level triggered */
> >  refclock = <38400>;
> >  refclock_xtal = <0>;
> >  };
> >
> > The macro idea sounds better to me, but in this case this code is so
> > simple that I don't think it's really worth it.
> >
> > And, from another point of view, I see this as only a specification of
> > the module's details.  The frequency value is not really used anywhere
> > outside the module, so we don't see it.  I don't think there's a good
> > reason to expose the actual frequency to the kernel (and parse it back
> > to the values the firmware needs), since nothing else inside the kernel
> > will care about it.
> Overview: we are adding bindings for 
> Documentation/devicetree/bindings/net/wireless/ti-wilink.txt Which I 
> believe is intended to be generic.
> 
> Current frequencies supported for tcxoclock is the following for WiLink7
> + 0 = 19.200 MHz
> + 1 = 26.000 MHz
> + 2 = 38.400 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon

On 06/27/2013 01:51 PM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 08:39 -0500, Nishanth Menon wrote:

On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho  wrote:

On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:

On 06/27/2013 08:19 AM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:

On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:

For the actual DTS files, I could add a wilink.dtsi with enumerations
for these values so they could be used in the node definitions.  But I'm
not sure it's going to be that valuable in the end.

The  way GPIO HIGH was defined might help to provide guidance I think :)


Where? As far as I can see, the GPIO flags are defined in a bitmap.


include/dt-bindings/gpio/gpio.h


Thanks! I don't see these macros used anywhere, though.

umm... I'd think you have'nt looked deep enough / lists :)


If you mean mailing lists, you're right, I didn't.  I just did a git
grep for the macros and didn't find any users.

git grep "GPIO_ACTIVE_[HIGH|LOW]" arch/arm/boot/dts/|wc -l
344
on next-20130626. anyways, besides the point.



This seems to be a completely different thing.  This is the header that
contains the helper functions to get GPIO-related device tree nodes,
isn't it?

That is true, but it also contains the flag for level which needs to
be in sync with the gpio.h dts header.

just a hint. not saying frequencies were defined in header. for systems
that define frequencies - example cpufreq OPPs, clock node usage, we do
not use indexing to frequency, instead, that is the responsibility of
driver to convert frequency back to required index.
git grep frequency Documentation/devicetree/bindings gives you how the
precedence looks like.

Personally, if given a choice, I'd go with actual frequencies rather
than indexes.


If I do that, I need to add also a separate flag to define whether the
XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
crystal; and 38.4MHz and 38.4MHz crystal...

Yes, you would have to. at the same time, it is easy for module maker
to provide dtsi corresponding to exact h/w representation on his
module using wilink chip without being worried about weird index value
whose meaning is hidden in binding


The module makers need to know about the bindings and read the document
before they specify the node in DTS.  I think for clarity, a comment in
the DTS file stating the actual frequency is good enough.  Simpler and
more efficient (in terms of DT binary size and module code size) than
adding the actual frequencies.



On the flip side, It also allows driver to reject frequencies /
configurations that are not supported by the corresponding chip.


It's actually much easier to reject frequencies that are not valid with
the enumeration.  "if (refclock > 5) { bail_out(); }".  If I need to
check every frequency, I need to add an array of valid frequencies and
so on.  Waste of code, IMHO.



As I said, just my 2 cents. Personally, I'd like dts files to be as
readable as c files without having to dig through bindings document.


As I said before, for readability, adding a comment with the frequency
is good enough.  This is what I did for PandaES's DTS (not sent out
yet):

wlan {
 compatible = "ti,wilink6";
 interrupt-parent = <>;
 interrupts = <21 0x4>;   /* gpio line 53, high level triggered 
*/
 refclock = <2>;  /* 38.4 MHz */
 };

Looks more readable to me than:

wlan {
 compatible = "ti,wilink6";
 interrupt-parent = <>;
 interrupts = <21 0x4>;   /* gpio line 53, high level triggered 
*/
 refclock = <38400>;
 refclock_xtal = <0>;
 };

The macro idea sounds better to me, but in this case this code is so
simple that I don't think it's really worth it.

And, from another point of view, I see this as only a specification of
the module's details.  The frequency value is not really used anywhere
outside the module, so we don't see it.  I don't think there's a good
reason to expose the actual frequency to the kernel (and parse it back
to the values the firmware needs), since nothing else inside the kernel
will care about it.
Overview: we are adding bindings for 
Documentation/devicetree/bindings/net/wireless/ti-wilink.txt Which I 
believe is intended to be generic.


Current frequencies supported for tcxoclock is the following for WiLink7
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
Say wilink9 comes along and redefines this map OR introduces support for 
20MHz support making the map 0-8, you'd no longer be able to support 
this map. or say a new update of firmware magically changes this mapping 
or something unexpected.


If the translation and validation is done in the driver, it is trivial 
to handle without redefining the 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 08:39 -0500, Nishanth Menon wrote:
> On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho  wrote:
> > On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
> >> On 06/27/2013 08:19 AM, Luciano Coelho wrote:
> >> > On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
> >> >> On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:
> >> >>> For the actual DTS files, I could add a wilink.dtsi with enumerations
> >> >>> for these values so they could be used in the node definitions.  But 
> >> >>> I'm
> >> >>> not sure it's going to be that valuable in the end.
> >> >> The  way GPIO HIGH was defined might help to provide guidance I think :)
> >> >
> >> > Where? As far as I can see, the GPIO flags are defined in a bitmap.
> >>
> >> include/dt-bindings/gpio/gpio.h
> >
> > Thanks! I don't see these macros used anywhere, though.
> umm... I'd think you have'nt looked deep enough / lists :)

If you mean mailing lists, you're right, I didn't.  I just did a git
grep for the macros and didn't find any users.


> >> And corresponding kernel header:
> >> include/linux/of_gpio.h
> >
> > This seems to be a completely different thing.  This is the header that
> > contains the helper functions to get GPIO-related device tree nodes,
> > isn't it?
> That is true, but it also contains the flag for level which needs to
> be in sync with the gpio.h dts header.
> >> just a hint. not saying frequencies were defined in header. for systems
> >> that define frequencies - example cpufreq OPPs, clock node usage, we do
> >> not use indexing to frequency, instead, that is the responsibility of
> >> driver to convert frequency back to required index.
> >> git grep frequency Documentation/devicetree/bindings gives you how the
> >> precedence looks like.
> >>
> >> Personally, if given a choice, I'd go with actual frequencies rather
> >> than indexes.
> >
> > If I do that, I need to add also a separate flag to define whether the
> > XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
> > crystal; and 38.4MHz and 38.4MHz crystal...
> Yes, you would have to. at the same time, it is easy for module maker
> to provide dtsi corresponding to exact h/w representation on his
> module using wilink chip without being worried about weird index value
> whose meaning is hidden in binding

The module makers need to know about the bindings and read the document
before they specify the node in DTS.  I think for clarity, a comment in
the DTS file stating the actual frequency is good enough.  Simpler and
more efficient (in terms of DT binary size and module code size) than
adding the actual frequencies.


> On the flip side, It also allows driver to reject frequencies /
> configurations that are not supported by the corresponding chip.

It's actually much easier to reject frequencies that are not valid with
the enumeration.  "if (refclock > 5) { bail_out(); }".  If I need to
check every frequency, I need to add an array of valid frequencies and
so on.  Waste of code, IMHO.


> As I said, just my 2 cents. Personally, I'd like dts files to be as
> readable as c files without having to dig through bindings document.

As I said before, for readability, adding a comment with the frequency
is good enough.  This is what I did for PandaES's DTS (not sent out
yet):

wlan {
 compatible = "ti,wilink6";
 interrupt-parent = <>;
 interrupts = <21 0x4>; /* gpio line 53, high level triggered */
 refclock = <2>;/* 38.4 MHz */
 };

Looks more readable to me than:

wlan {
 compatible = "ti,wilink6";
 interrupt-parent = <>;
 interrupts = <21 0x4>; /* gpio line 53, high level triggered */
 refclock = <38400>;
 refclock_xtal = <0>;
 };

The macro idea sounds better to me, but in this case this code is so
simple that I don't think it's really worth it.

And, from another point of view, I see this as only a specification of
the module's details.  The frequency value is not really used anywhere
outside the module, so we don't see it.  I don't think there's a good
reason to expose the actual frequency to the kernel (and parse it back
to the values the firmware needs), since nothing else inside the kernel
will care about it.

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon
On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho  wrote:
> On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
>> On 06/27/2013 08:19 AM, Luciano Coelho wrote:
>> > On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
>> >> On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:
>> >>> For the actual DTS files, I could add a wilink.dtsi with enumerations
>> >>> for these values so they could be used in the node definitions.  But I'm
>> >>> not sure it's going to be that valuable in the end.
>> >> The  way GPIO HIGH was defined might help to provide guidance I think :)
>> >
>> > Where? As far as I can see, the GPIO flags are defined in a bitmap.
>>
>> include/dt-bindings/gpio/gpio.h
>
> Thanks! I don't see these macros used anywhere, though.
umm... I'd think you have'nt looked deep enough / lists :)

>
>> And corresponding kernel header:
>> include/linux/of_gpio.h
>
> This seems to be a completely different thing.  This is the header that
> contains the helper functions to get GPIO-related device tree nodes,
> isn't it?
That is true, but it also contains the flag for level which needs to
be in sync with the gpio.h dts header.
>> just a hint. not saying frequencies were defined in header. for systems
>> that define frequencies - example cpufreq OPPs, clock node usage, we do
>> not use indexing to frequency, instead, that is the responsibility of
>> driver to convert frequency back to required index.
>> git grep frequency Documentation/devicetree/bindings gives you how the
>> precedence looks like.
>>
>> Personally, if given a choice, I'd go with actual frequencies rather
>> than indexes.
>
> If I do that, I need to add also a separate flag to define whether the
> XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
> crystal; and 38.4MHz and 38.4MHz crystal...
Yes, you would have to. at the same time, it is easy for module maker
to provide dtsi corresponding to exact h/w representation on his
module using wilink chip without being worried about weird index value
whose meaning is hidden in binding
On the flip side, It also allows driver to reject frequencies /
configurations that are not supported by the corresponding chip.

As I said, just my 2 cents. Personally, I'd like dts files to be as
readable as c files without having to dig through bindings document.

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
> On 06/27/2013 08:19 AM, Luciano Coelho wrote:
> > On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
> >> On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:
> >>> For the actual DTS files, I could add a wilink.dtsi with enumerations
> >>> for these values so they could be used in the node definitions.  But I'm
> >>> not sure it's going to be that valuable in the end.
> >> The  way GPIO HIGH was defined might help to provide guidance I think :)
> >
> > Where? As far as I can see, the GPIO flags are defined in a bitmap.
> 
> include/dt-bindings/gpio/gpio.h

Thanks! I don't see these macros used anywhere, though.

> And corresponding kernel header:
> include/linux/of_gpio.h

This seems to be a completely different thing.  This is the header that
contains the helper functions to get GPIO-related device tree nodes,
isn't it?


> just a hint. not saying frequencies were defined in header. for systems 
> that define frequencies - example cpufreq OPPs, clock node usage, we do 
> not use indexing to frequency, instead, that is the responsibility of 
> driver to convert frequency back to required index.
> git grep frequency Documentation/devicetree/bindings gives you how the 
> precedence looks like.
> 
> Personally, if given a choice, I'd go with actual frequencies rather 
> than indexes.

If I do that, I need to add also a separate flag to define whether the
XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
crystal; and 38.4MHz and 38.4MHz crystal...

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon

On 06/27/2013 08:19 AM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:

On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:

For the actual DTS files, I could add a wilink.dtsi with enumerations
for these values so they could be used in the node definitions.  But I'm
not sure it's going to be that valuable in the end.

The  way GPIO HIGH was defined might help to provide guidance I think :)


Where? As far as I can see, the GPIO flags are defined in a bitmap.


include/dt-bindings/gpio/gpio.h
And corresponding kernel header:
include/linux/of_gpio.h

just a hint. not saying frequencies were defined in header. for systems 
that define frequencies - example cpufreq OPPs, clock node usage, we do 
not use indexing to frequency, instead, that is the responsibility of 
driver to convert frequency back to required index.
git grep frequency Documentation/devicetree/bindings gives you how the 
precedence looks like.


Personally, if given a choice, I'd go with actual frequencies rather 
than indexes.


--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
> On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:
> > For the actual DTS files, I could add a wilink.dtsi with enumerations
> > for these values so they could be used in the node definitions.  But I'm
> > not sure it's going to be that valuable in the end.
> The  way GPIO HIGH was defined might help to provide guidance I think :)

Where? As far as I can see, the GPIO flags are defined in a bitmap.

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon
On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho  wrote:
> On Thu, 2013-06-27 at 07:51 -0500, Nishanth Menon wrote:
>> On 11:47-20130627, Luciano Coelho wrote:
>> > (added mailing lists and everyone back to the thread)
>> >
>> > On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
>> > > On 06/25/2013 03:35 AM, Luciano Coelho wrote:
>> > > > +Optional properties:
>> > > > +
>> > > > +
>> > > > +- refclock: the internal WLAN reference clock frequency (required for
>> > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
>> > > > +  following:
>> > > > +   0 = 19.2 MHz
>> > > > +   1 = 26.0 MHz
>> > > > +   2 = 38.4 MHz
>> > > > +   3 = 52.0 MHz
>> > > > +   4 = 38.4 MHz, XTAL
>> > > > +   5 = 26.0 MHz, XTAL
>> > > > +
>> > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
>> > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
>> > > > +  following:
>> > > > +   0 = 19.200 MHz
>> > > > +   1 = 26.000 MHz
>> > > > +   2 = 38.400 MHz
>> > > > +   3 = 52.000 MHz
>> > > > +   4 = 16.368 MHz
>> > > > +   5 = 32.736 MHz
>> > > > +   6 = 16.800 MHz
>> > > > +   7 = 33.600 MHz
>> > > >
>> > > just a gentle query - why not use frequency itself here in Hz for
>> > > refclock and txoclk?
>> >
>> > I thought about using the actual frequencies, but I decided not to do
>> > so, because I'd have to convert them to these values anyway.  These
>> > values are used to configure the firmware and it uses these
>> > "enumerations".
>> Could we not hide this under preprocessor macros instead? just wondering
>> of txoclock = <6>; kind of usage.. easy to make mistakes and easier to
>> confuse a new reader :).
>
> Yes, I guess we could create some preprocessor macros for this.  But the
> documentation would remain the same.  I can't add preprocessor macros to
> the bindings documentation. ;)
>
> For the actual DTS files, I could add a wilink.dtsi with enumerations
> for these values so they could be used in the node definitions.  But I'm
> not sure it's going to be that valuable in the end.
The  way GPIO HIGH was defined might help to provide guidance I think :)

Regards,
NM
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 07:51 -0500, Nishanth Menon wrote:
> On 11:47-20130627, Luciano Coelho wrote:
> > (added mailing lists and everyone back to the thread)
> > 
> > On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
> > > On 06/25/2013 03:35 AM, Luciano Coelho wrote:
> > > > +Optional properties:
> > > > +
> > > > +
> > > > +- refclock: the internal WLAN reference clock frequency (required for
> > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > > +  following:
> > > > +   0 = 19.2 MHz
> > > > +   1 = 26.0 MHz
> > > > +   2 = 38.4 MHz
> > > > +   3 = 52.0 MHz
> > > > +   4 = 38.4 MHz, XTAL
> > > > +   5 = 26.0 MHz, XTAL
> > > > +
> > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > > +  following:
> > > > +   0 = 19.200 MHz
> > > > +   1 = 26.000 MHz
> > > > +   2 = 38.400 MHz
> > > > +   3 = 52.000 MHz
> > > > +   4 = 16.368 MHz
> > > > +   5 = 32.736 MHz
> > > > +   6 = 16.800 MHz
> > > > +   7 = 33.600 MHz
> > > >
> > > just a gentle query - why not use frequency itself here in Hz for 
> > > refclock and txoclk?
> > 
> > I thought about using the actual frequencies, but I decided not to do
> > so, because I'd have to convert them to these values anyway.  These
> > values are used to configure the firmware and it uses these
> > "enumerations".
> Could we not hide this under preprocessor macros instead? just wondering
> of txoclock = <6>; kind of usage.. easy to make mistakes and easier to
> confuse a new reader :).

Yes, I guess we could create some preprocessor macros for this.  But the
documentation would remain the same.  I can't add preprocessor macros to
the bindings documentation. ;)

For the actual DTS files, I could add a wilink.dtsi with enumerations
for these values so they could be used in the node definitions.  But I'm
not sure it's going to be that valuable in the end.

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon
On 11:47-20130627, Luciano Coelho wrote:
> (added mailing lists and everyone back to the thread)
> 
> On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
> > On 06/25/2013 03:35 AM, Luciano Coelho wrote:
> > > +Optional properties:
> > > +
> > > +
> > > +- refclock: the internal WLAN reference clock frequency (required for
> > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.2 MHz
> > > + 1 = 26.0 MHz
> > > + 2 = 38.4 MHz
> > > + 3 = 52.0 MHz
> > > + 4 = 38.4 MHz, XTAL
> > > + 5 = 26.0 MHz, XTAL
> > > +
> > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.200 MHz
> > > + 1 = 26.000 MHz
> > > + 2 = 38.400 MHz
> > > + 3 = 52.000 MHz
> > > + 4 = 16.368 MHz
> > > + 5 = 32.736 MHz
> > > + 6 = 16.800 MHz
> > > + 7 = 33.600 MHz
> > >
> > just a gentle query - why not use frequency itself here in Hz for 
> > refclock and txoclk?
> 
> I thought about using the actual frequencies, but I decided not to do
> so, because I'd have to convert them to these values anyway.  These
> values are used to configure the firmware and it uses these
> "enumerations".
Could we not hide this under preprocessor macros instead? just wondering
of txoclock = <6>; kind of usage.. easy to make mistakes and easier to
confuse a new reader :).

just my 2 cents.
> 
> 
> > might not another option of using
> > node {
> > clocks=<>;
> > }
> > 
> > Usually refclock is an external clock source, no?
> 
> No.  In the WiLink case, both refclock and tcxoclock are internal
> clocks.  They are in the module itself and what we need to do is tell
> the WiLink chip what the module's clocks look like.
> 
> 
> > the above allows you to do an devm_clk_get and clk_get_rate() to figure 
> > out the exact clock frequency.
> 
> No, we can't use these calls, because they are internal clocks.
> 
> Please see my more complete explanation as an answer to Tony's email.
K thanks.
> 
> Thanks for your review!

Glad to be of help.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
(added mailing lists and everyone back to the thread)

On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
> On 06/25/2013 03:35 AM, Luciano Coelho wrote:
> > +Optional properties:
> > +
> > +
> > +- refclock: the internal WLAN reference clock frequency (required for
> > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.2 MHz
> > +   1 = 26.0 MHz
> > +   2 = 38.4 MHz
> > +   3 = 52.0 MHz
> > +   4 = 38.4 MHz, XTAL
> > +   5 = 26.0 MHz, XTAL
> > +
> > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.200 MHz
> > +   1 = 26.000 MHz
> > +   2 = 38.400 MHz
> > +   3 = 52.000 MHz
> > +   4 = 16.368 MHz
> > +   5 = 32.736 MHz
> > +   6 = 16.800 MHz
> > +   7 = 33.600 MHz
> >
> just a gentle query - why not use frequency itself here in Hz for 
> refclock and txoclk?

I thought about using the actual frequencies, but I decided not to do
so, because I'd have to convert them to these values anyway.  These
values are used to configure the firmware and it uses these
"enumerations".


> might not another option of using
> node {
> clocks=<>;
> }
> 
> Usually refclock is an external clock source, no?

No.  In the WiLink case, both refclock and tcxoclock are internal
clocks.  They are in the module itself and what we need to do is tell
the WiLink chip what the module's clocks look like.


> the above allows you to do an devm_clk_get and clk_get_rate() to figure 
> out the exact clock frequency.

No, we can't use these calls, because they are internal clocks.

Please see my more complete explanation as an answer to Tony's email.

Thanks for your review!

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
(added mailing lists and everyone back to the thread)

On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
 On 06/25/2013 03:35 AM, Luciano Coelho wrote:
  +Optional properties:
  +
  +
  +- refclock: the internal WLAN reference clock frequency (required for
  +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
  +  following:
  +   0 = 19.2 MHz
  +   1 = 26.0 MHz
  +   2 = 38.4 MHz
  +   3 = 52.0 MHz
  +   4 = 38.4 MHz, XTAL
  +   5 = 26.0 MHz, XTAL
  +
  +- tcxoclock: the internal WLAN TCXO clock frequency (required for
  +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
  +  following:
  +   0 = 19.200 MHz
  +   1 = 26.000 MHz
  +   2 = 38.400 MHz
  +   3 = 52.000 MHz
  +   4 = 16.368 MHz
  +   5 = 32.736 MHz
  +   6 = 16.800 MHz
  +   7 = 33.600 MHz
 
 just a gentle query - why not use frequency itself here in Hz for 
 refclock and txoclk?

I thought about using the actual frequencies, but I decided not to do
so, because I'd have to convert them to these values anyway.  These
values are used to configure the firmware and it uses these
enumerations.


 might not another option of using
 node {
 clocks=clk;
 }
 
 Usually refclock is an external clock source, no?

No.  In the WiLink case, both refclock and tcxoclock are internal
clocks.  They are in the module itself and what we need to do is tell
the WiLink chip what the module's clocks look like.


 the above allows you to do an devm_clk_get and clk_get_rate() to figure 
 out the exact clock frequency.

No, we can't use these calls, because they are internal clocks.

Please see my more complete explanation as an answer to Tony's email.

Thanks for your review!

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon
On 11:47-20130627, Luciano Coelho wrote:
 (added mailing lists and everyone back to the thread)
 
 On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
  On 06/25/2013 03:35 AM, Luciano Coelho wrote:
   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
   +
   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  just a gentle query - why not use frequency itself here in Hz for 
  refclock and txoclk?
 
 I thought about using the actual frequencies, but I decided not to do
 so, because I'd have to convert them to these values anyway.  These
 values are used to configure the firmware and it uses these
 enumerations.
Could we not hide this under preprocessor macros instead? just wondering
of txoclock = 6; kind of usage.. easy to make mistakes and easier to
confuse a new reader :).

just my 2 cents.
 
 
  might not another option of using
  node {
  clocks=clk;
  }
  
  Usually refclock is an external clock source, no?
 
 No.  In the WiLink case, both refclock and tcxoclock are internal
 clocks.  They are in the module itself and what we need to do is tell
 the WiLink chip what the module's clocks look like.
 
 
  the above allows you to do an devm_clk_get and clk_get_rate() to figure 
  out the exact clock frequency.
 
 No, we can't use these calls, because they are internal clocks.
 
 Please see my more complete explanation as an answer to Tony's email.
K thanks.
 
 Thanks for your review!

Glad to be of help.

-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 07:51 -0500, Nishanth Menon wrote:
 On 11:47-20130627, Luciano Coelho wrote:
  (added mailing lists and everyone back to the thread)
  
  On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
   On 06/25/2013 03:35 AM, Luciano Coelho wrote:
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
   
   just a gentle query - why not use frequency itself here in Hz for 
   refclock and txoclk?
  
  I thought about using the actual frequencies, but I decided not to do
  so, because I'd have to convert them to these values anyway.  These
  values are used to configure the firmware and it uses these
  enumerations.
 Could we not hide this under preprocessor macros instead? just wondering
 of txoclock = 6; kind of usage.. easy to make mistakes and easier to
 confuse a new reader :).

Yes, I guess we could create some preprocessor macros for this.  But the
documentation would remain the same.  I can't add preprocessor macros to
the bindings documentation. ;)

For the actual DTS files, I could add a wilink.dtsi with enumerations
for these values so they could be used in the node definitions.  But I'm
not sure it's going to be that valuable in the end.

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon
On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:
 On Thu, 2013-06-27 at 07:51 -0500, Nishanth Menon wrote:
 On 11:47-20130627, Luciano Coelho wrote:
  (added mailing lists and everyone back to the thread)
 
  On Wed, 2013-06-26 at 23:38 -0500, Nishanth Menon wrote:
   On 06/25/2013 03:35 AM, Luciano Coelho wrote:
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
   
   just a gentle query - why not use frequency itself here in Hz for
   refclock and txoclk?
 
  I thought about using the actual frequencies, but I decided not to do
  so, because I'd have to convert them to these values anyway.  These
  values are used to configure the firmware and it uses these
  enumerations.
 Could we not hide this under preprocessor macros instead? just wondering
 of txoclock = 6; kind of usage.. easy to make mistakes and easier to
 confuse a new reader :).

 Yes, I guess we could create some preprocessor macros for this.  But the
 documentation would remain the same.  I can't add preprocessor macros to
 the bindings documentation. ;)

 For the actual DTS files, I could add a wilink.dtsi with enumerations
 for these values so they could be used in the node definitions.  But I'm
 not sure it's going to be that valuable in the end.
The  way GPIO HIGH was defined might help to provide guidance I think :)

Regards,
NM
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
 On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:
  For the actual DTS files, I could add a wilink.dtsi with enumerations
  for these values so they could be used in the node definitions.  But I'm
  not sure it's going to be that valuable in the end.
 The  way GPIO HIGH was defined might help to provide guidance I think :)

Where? As far as I can see, the GPIO flags are defined in a bitmap.

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon

On 06/27/2013 08:19 AM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:

On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:

For the actual DTS files, I could add a wilink.dtsi with enumerations
for these values so they could be used in the node definitions.  But I'm
not sure it's going to be that valuable in the end.

The  way GPIO HIGH was defined might help to provide guidance I think :)


Where? As far as I can see, the GPIO flags are defined in a bitmap.


include/dt-bindings/gpio/gpio.h
And corresponding kernel header:
include/linux/of_gpio.h

just a hint. not saying frequencies were defined in header. for systems 
that define frequencies - example cpufreq OPPs, clock node usage, we do 
not use indexing to frequency, instead, that is the responsibility of 
driver to convert frequency back to required index.
git grep frequency Documentation/devicetree/bindings gives you how the 
precedence looks like.


Personally, if given a choice, I'd go with actual frequencies rather 
than indexes.


--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
 On 06/27/2013 08:19 AM, Luciano Coelho wrote:
  On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
  On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:
  For the actual DTS files, I could add a wilink.dtsi with enumerations
  for these values so they could be used in the node definitions.  But I'm
  not sure it's going to be that valuable in the end.
  The  way GPIO HIGH was defined might help to provide guidance I think :)
 
  Where? As far as I can see, the GPIO flags are defined in a bitmap.
 
 include/dt-bindings/gpio/gpio.h

Thanks! I don't see these macros used anywhere, though.

 And corresponding kernel header:
 include/linux/of_gpio.h

This seems to be a completely different thing.  This is the header that
contains the helper functions to get GPIO-related device tree nodes,
isn't it?


 just a hint. not saying frequencies were defined in header. for systems 
 that define frequencies - example cpufreq OPPs, clock node usage, we do 
 not use indexing to frequency, instead, that is the responsibility of 
 driver to convert frequency back to required index.
 git grep frequency Documentation/devicetree/bindings gives you how the 
 precedence looks like.
 
 Personally, if given a choice, I'd go with actual frequencies rather 
 than indexes.

If I do that, I need to add also a separate flag to define whether the
XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
crystal; and 38.4MHz and 38.4MHz crystal...

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon
On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho coe...@ti.com wrote:
 On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
 On 06/27/2013 08:19 AM, Luciano Coelho wrote:
  On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
  On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:
  For the actual DTS files, I could add a wilink.dtsi with enumerations
  for these values so they could be used in the node definitions.  But I'm
  not sure it's going to be that valuable in the end.
  The  way GPIO HIGH was defined might help to provide guidance I think :)
 
  Where? As far as I can see, the GPIO flags are defined in a bitmap.

 include/dt-bindings/gpio/gpio.h

 Thanks! I don't see these macros used anywhere, though.
umm... I'd think you have'nt looked deep enough / lists :)


 And corresponding kernel header:
 include/linux/of_gpio.h

 This seems to be a completely different thing.  This is the header that
 contains the helper functions to get GPIO-related device tree nodes,
 isn't it?
That is true, but it also contains the flag for level which needs to
be in sync with the gpio.h dts header.
 just a hint. not saying frequencies were defined in header. for systems
 that define frequencies - example cpufreq OPPs, clock node usage, we do
 not use indexing to frequency, instead, that is the responsibility of
 driver to convert frequency back to required index.
 git grep frequency Documentation/devicetree/bindings gives you how the
 precedence looks like.

 Personally, if given a choice, I'd go with actual frequencies rather
 than indexes.

 If I do that, I need to add also a separate flag to define whether the
 XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
 crystal; and 38.4MHz and 38.4MHz crystal...
Yes, you would have to. at the same time, it is easy for module maker
to provide dtsi corresponding to exact h/w representation on his
module using wilink chip without being worried about weird index value
whose meaning is hidden in binding
On the flip side, It also allows driver to reject frequencies /
configurations that are not supported by the corresponding chip.

As I said, just my 2 cents. Personally, I'd like dts files to be as
readable as c files without having to dig through bindings document.

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 08:39 -0500, Nishanth Menon wrote:
 On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho coe...@ti.com wrote:
  On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
  On 06/27/2013 08:19 AM, Luciano Coelho wrote:
   On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
   On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:
   For the actual DTS files, I could add a wilink.dtsi with enumerations
   for these values so they could be used in the node definitions.  But 
   I'm
   not sure it's going to be that valuable in the end.
   The  way GPIO HIGH was defined might help to provide guidance I think :)
  
   Where? As far as I can see, the GPIO flags are defined in a bitmap.
 
  include/dt-bindings/gpio/gpio.h
 
  Thanks! I don't see these macros used anywhere, though.
 umm... I'd think you have'nt looked deep enough / lists :)

If you mean mailing lists, you're right, I didn't.  I just did a git
grep for the macros and didn't find any users.


  And corresponding kernel header:
  include/linux/of_gpio.h
 
  This seems to be a completely different thing.  This is the header that
  contains the helper functions to get GPIO-related device tree nodes,
  isn't it?
 That is true, but it also contains the flag for level which needs to
 be in sync with the gpio.h dts header.
  just a hint. not saying frequencies were defined in header. for systems
  that define frequencies - example cpufreq OPPs, clock node usage, we do
  not use indexing to frequency, instead, that is the responsibility of
  driver to convert frequency back to required index.
  git grep frequency Documentation/devicetree/bindings gives you how the
  precedence looks like.
 
  Personally, if given a choice, I'd go with actual frequencies rather
  than indexes.
 
  If I do that, I need to add also a separate flag to define whether the
  XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
  crystal; and 38.4MHz and 38.4MHz crystal...
 Yes, you would have to. at the same time, it is easy for module maker
 to provide dtsi corresponding to exact h/w representation on his
 module using wilink chip without being worried about weird index value
 whose meaning is hidden in binding

The module makers need to know about the bindings and read the document
before they specify the node in DTS.  I think for clarity, a comment in
the DTS file stating the actual frequency is good enough.  Simpler and
more efficient (in terms of DT binary size and module code size) than
adding the actual frequencies.


 On the flip side, It also allows driver to reject frequencies /
 configurations that are not supported by the corresponding chip.

It's actually much easier to reject frequencies that are not valid with
the enumeration.  if (refclock  5) { bail_out(); }.  If I need to
check every frequency, I need to add an array of valid frequencies and
so on.  Waste of code, IMHO.


 As I said, just my 2 cents. Personally, I'd like dts files to be as
 readable as c files without having to dig through bindings document.

As I said before, for readability, adding a comment with the frequency
is good enough.  This is what I did for PandaES's DTS (not sent out
yet):

wlan {
 compatible = ti,wilink6;
 interrupt-parent = gpio2;
 interrupts = 21 0x4; /* gpio line 53, high level triggered */
 refclock = 2;/* 38.4 MHz */
 };

Looks more readable to me than:

wlan {
 compatible = ti,wilink6;
 interrupt-parent = gpio2;
 interrupts = 21 0x4; /* gpio line 53, high level triggered */
 refclock = 38400;
 refclock_xtal = 0;
 };

The macro idea sounds better to me, but in this case this code is so
simple that I don't think it's really worth it.

And, from another point of view, I see this as only a specification of
the module's details.  The frequency value is not really used anywhere
outside the module, so we don't see it.  I don't think there's a good
reason to expose the actual frequency to the kernel (and parse it back
to the values the firmware needs), since nothing else inside the kernel
will care about it.

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon

On 06/27/2013 01:51 PM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 08:39 -0500, Nishanth Menon wrote:

On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho coe...@ti.com wrote:

On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:

On 06/27/2013 08:19 AM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:

On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:

For the actual DTS files, I could add a wilink.dtsi with enumerations
for these values so they could be used in the node definitions.  But I'm
not sure it's going to be that valuable in the end.

The  way GPIO HIGH was defined might help to provide guidance I think :)


Where? As far as I can see, the GPIO flags are defined in a bitmap.


include/dt-bindings/gpio/gpio.h


Thanks! I don't see these macros used anywhere, though.

umm... I'd think you have'nt looked deep enough / lists :)


If you mean mailing lists, you're right, I didn't.  I just did a git
grep for the macros and didn't find any users.

git grep GPIO_ACTIVE_[HIGH|LOW] arch/arm/boot/dts/|wc -l
344
on next-20130626. anyways, besides the point.



This seems to be a completely different thing.  This is the header that
contains the helper functions to get GPIO-related device tree nodes,
isn't it?

That is true, but it also contains the flag for level which needs to
be in sync with the gpio.h dts header.

just a hint. not saying frequencies were defined in header. for systems
that define frequencies - example cpufreq OPPs, clock node usage, we do
not use indexing to frequency, instead, that is the responsibility of
driver to convert frequency back to required index.
git grep frequency Documentation/devicetree/bindings gives you how the
precedence looks like.

Personally, if given a choice, I'd go with actual frequencies rather
than indexes.


If I do that, I need to add also a separate flag to define whether the
XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
crystal; and 38.4MHz and 38.4MHz crystal...

Yes, you would have to. at the same time, it is easy for module maker
to provide dtsi corresponding to exact h/w representation on his
module using wilink chip without being worried about weird index value
whose meaning is hidden in binding


The module makers need to know about the bindings and read the document
before they specify the node in DTS.  I think for clarity, a comment in
the DTS file stating the actual frequency is good enough.  Simpler and
more efficient (in terms of DT binary size and module code size) than
adding the actual frequencies.



On the flip side, It also allows driver to reject frequencies /
configurations that are not supported by the corresponding chip.


It's actually much easier to reject frequencies that are not valid with
the enumeration.  if (refclock  5) { bail_out(); }.  If I need to
check every frequency, I need to add an array of valid frequencies and
so on.  Waste of code, IMHO.



As I said, just my 2 cents. Personally, I'd like dts files to be as
readable as c files without having to dig through bindings document.


As I said before, for readability, adding a comment with the frequency
is good enough.  This is what I did for PandaES's DTS (not sent out
yet):

wlan {
 compatible = ti,wilink6;
 interrupt-parent = gpio2;
 interrupts = 21 0x4;   /* gpio line 53, high level triggered 
*/
 refclock = 2;  /* 38.4 MHz */
 };

Looks more readable to me than:

wlan {
 compatible = ti,wilink6;
 interrupt-parent = gpio2;
 interrupts = 21 0x4;   /* gpio line 53, high level triggered 
*/
 refclock = 38400;
 refclock_xtal = 0;
 };

The macro idea sounds better to me, but in this case this code is so
simple that I don't think it's really worth it.

And, from another point of view, I see this as only a specification of
the module's details.  The frequency value is not really used anywhere
outside the module, so we don't see it.  I don't think there's a good
reason to expose the actual frequency to the kernel (and parse it back
to the values the firmware needs), since nothing else inside the kernel
will care about it.
Overview: we are adding bindings for 
Documentation/devicetree/bindings/net/wireless/ti-wilink.txt Which I 
believe is intended to be generic.


Current frequencies supported for tcxoclock is the following for WiLink7
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
Say wilink9 comes along and redefines this map OR introduces support for 
20MHz support making the map 0-8, you'd no longer be able to support 
this map. or say a new update of firmware magically changes this mapping 
or something unexpected.


If the translation and validation is done in the driver, it is trivial 
to handle without 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Luciano Coelho
On Thu, 2013-06-27 at 14:12 -0500, Nishanth Menon wrote:
 On 06/27/2013 01:51 PM, Luciano Coelho wrote:
  On Thu, 2013-06-27 at 08:39 -0500, Nishanth Menon wrote:
  On Thu, Jun 27, 2013 at 8:30 AM, Luciano Coelho coe...@ti.com wrote:
  On Thu, 2013-06-27 at 08:23 -0500, Nishanth Menon wrote:
  On 06/27/2013 08:19 AM, Luciano Coelho wrote:
  On Thu, 2013-06-27 at 08:15 -0500, Nishanth Menon wrote:
  On Thu, Jun 27, 2013 at 7:58 AM, Luciano Coelho coe...@ti.com wrote:
  For the actual DTS files, I could add a wilink.dtsi with enumerations
  for these values so they could be used in the node definitions.  But 
  I'm
  not sure it's going to be that valuable in the end.
  The  way GPIO HIGH was defined might help to provide guidance I think 
  :)
 
  Where? As far as I can see, the GPIO flags are defined in a bitmap.
 
  include/dt-bindings/gpio/gpio.h
 
  Thanks! I don't see these macros used anywhere, though.
  umm... I'd think you have'nt looked deep enough / lists :)
 
  If you mean mailing lists, you're right, I didn't.  I just did a git
  grep for the macros and didn't find any users.
 git grep GPIO_ACTIVE_[HIGH|LOW] arch/arm/boot/dts/|wc -l
 344
 on next-20130626. anyways, besides the point.
 
 
  This seems to be a completely different thing.  This is the header that
  contains the helper functions to get GPIO-related device tree nodes,
  isn't it?
  That is true, but it also contains the flag for level which needs to
  be in sync with the gpio.h dts header.
  just a hint. not saying frequencies were defined in header. for systems
  that define frequencies - example cpufreq OPPs, clock node usage, we do
  not use indexing to frequency, instead, that is the responsibility of
  driver to convert frequency back to required index.
  git grep frequency Documentation/devicetree/bindings gives you how the
  precedence looks like.
 
  Personally, if given a choice, I'd go with actual frequencies rather
  than indexes.
 
  If I do that, I need to add also a separate flag to define whether the
  XTAL clock is used or not.  For instance, we have 26MHz and 26MHz
  crystal; and 38.4MHz and 38.4MHz crystal...
  Yes, you would have to. at the same time, it is easy for module maker
  to provide dtsi corresponding to exact h/w representation on his
  module using wilink chip without being worried about weird index value
  whose meaning is hidden in binding
 
  The module makers need to know about the bindings and read the document
  before they specify the node in DTS.  I think for clarity, a comment in
  the DTS file stating the actual frequency is good enough.  Simpler and
  more efficient (in terms of DT binary size and module code size) than
  adding the actual frequencies.
 
 
  On the flip side, It also allows driver to reject frequencies /
  configurations that are not supported by the corresponding chip.
 
  It's actually much easier to reject frequencies that are not valid with
  the enumeration.  if (refclock  5) { bail_out(); }.  If I need to
  check every frequency, I need to add an array of valid frequencies and
  so on.  Waste of code, IMHO.
 
 
  As I said, just my 2 cents. Personally, I'd like dts files to be as
  readable as c files without having to dig through bindings document.
 
  As I said before, for readability, adding a comment with the frequency
  is good enough.  This is what I did for PandaES's DTS (not sent out
  yet):
 
  wlan {
   compatible = ti,wilink6;
   interrupt-parent = gpio2;
   interrupts = 21 0x4; /* gpio line 53, high level triggered */
   refclock = 2;/* 38.4 MHz */
   };
 
  Looks more readable to me than:
 
  wlan {
   compatible = ti,wilink6;
   interrupt-parent = gpio2;
   interrupts = 21 0x4; /* gpio line 53, high level triggered */
   refclock = 38400;
   refclock_xtal = 0;
   };
 
  The macro idea sounds better to me, but in this case this code is so
  simple that I don't think it's really worth it.
 
  And, from another point of view, I see this as only a specification of
  the module's details.  The frequency value is not really used anywhere
  outside the module, so we don't see it.  I don't think there's a good
  reason to expose the actual frequency to the kernel (and parse it back
  to the values the firmware needs), since nothing else inside the kernel
  will care about it.
 Overview: we are adding bindings for 
 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt Which I 
 believe is intended to be generic.
 
 Current frequencies supported for tcxoclock is the following for WiLink7
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz
 Say wilink9 comes along and redefines this map OR introduces support for 
 20MHz support making the map 0-8, you'd no longer be able to support 
 this map. or say a new update of firmware 

Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-27 Thread Nishanth Menon

On 06/27/2013 02:46 PM, Luciano Coelho wrote:

On Thu, 2013-06-27 at 14:12 -0500, Nishanth Menon wrote:

[...]

Indexes to another entity is always a maintenance burden in the longer
run and needs judicious control. If it is possible to avoid it by
selecting the right parameters, I am a hard advocate for the same.


I tend to agree.  But you need a balance.  In theory you're right.  But
I think if you take the real world example, it is over-engineering.

Anyway, if you *really* think this needs to be changed, I think we're in
a deadlock here and I'd like to hear other people's opinions.  I don't
mind making the change, but I'm still not convinced it is worth it,
since it just adds complexity.

And hey, this is too much bikeshedding for such a small detail.

Lol :)

Alrite, if no one else is complaining, I am not going to block it either.

--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-26 Thread Tony Lindgren
* Luciano Coelho  [130626 01:19]:
> Hi Tony,
> 
> On Tue, 2013-06-25 at 23:24 -0700, Tony Lindgren wrote:
> > * Luciano Coelho  [130625 12:43]:
> > > On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
> > > > Add device tree bindings documentation for the TI WiLink modules.
> > > > Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> > > > modules is supported.
> > > > 
> > > > Signed-off-by: Luciano Coelho 
> > > > ---
> 
> [...]
> 
> > > > +Optional properties:
> > > > +
> > > > +
> > > > +- refclock: the internal WLAN reference clock frequency (required for
> > > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > > +  following:
> > > > +   0 = 19.2 MHz
> > > > +   1 = 26.0 MHz
> > > > +   2 = 38.4 MHz
> > > > +   3 = 52.0 MHz
> > > > +   4 = 38.4 MHz, XTAL
> > > > +   5 = 26.0 MHz, XTAL
> > 
> > This is just the omap refclock, right? If so, you can just pass the
> > standard clock phandle. I know we don't yet have the DT clocks merged,
> > but Tero just posted another revision of those.
> 
> This is an internal clock.  This clock is part of the module that
> contains the WiLink chip.  It is not associated with the clocks in the
> main board (OMAP).
> 
> 
> > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > > +  following:
> > > > +   0 = 19.200 MHz
> > > > +   1 = 26.000 MHz
> > > > +   2 = 38.400 MHz
> > > > +   3 = 52.000 MHz
> > > > +   4 = 16.368 MHz
> > > > +   5 = 32.736 MHz
> > > > +   6 = 16.800 MHz
> > > > +   7 = 33.600 MHz
> > 
> > Where does this clock come from? Maybe this can be set based on the
> > compatible value if it's completely internal?
> 
> This is also a completely internal clock.  My "compatible" values are
> based on the WiLink chip itself, not in the module that contains the
> chip.  There are several modules and they are the ones that specify the
> clock frequencies.  This data I'm passing here is just to tell the
> WiLink chip which frequencies the module uses.
> 
> My driver is for the WiLink chip itself, not to the module (in theory).
> So I think having the WiLink values as bindings would be more generic
> than having to specify values for each available module (eg.
> "lsr-research,tiwi-ble") and mapping those values to specific
> frequencies in the driver.
> 
>  
> > > If this is okay for everyone, can I push this via my tree (which goes to
> > > linux-wireless->net->linus)? I think it makes more sense to send the
> > > documentation together with the patch that actually implements the DT
> > > node parsing in the driver.
> > 
> > If we can use the standard bindings, it might be worth waiting until
> > we have the DT clocks available as we have the pdata workaround merged
> > anyways. That's because then we don't need to support the custom
> > binding later on ;)
> 
> I looked into Tero's patches and I considered using the generic clock
> bindings, but I think it doesn't make sense in this case.  The thing is
> that the module is not providing the clocks to the main board.  Neither
> is the WiLink chip consuming clocks from the main board.
> 
> I thought about specifying clock providers and consumers to be used only
> by the module and WiLink chip, but I think it's overkill.  And we would
> also have to find a way to prevent the main clock framework from trying
> to handle them.
> 
> So, my conclusion was that, even though these *are* clocks, from the
> main board's perspective they're just specifications of what the module
> looks like.
> 
> Does this make sense?

OK yes, in that case looks fine to me:

Acked-by: Tony Lindgren 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-26 Thread Luciano Coelho
Hi Tony,

On Tue, 2013-06-25 at 23:24 -0700, Tony Lindgren wrote:
> * Luciano Coelho  [130625 12:43]:
> > On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
> > > Add device tree bindings documentation for the TI WiLink modules.
> > > Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> > > modules is supported.
> > > 
> > > Signed-off-by: Luciano Coelho 
> > > ---

[...]

> > > +Optional properties:
> > > +
> > > +
> > > +- refclock: the internal WLAN reference clock frequency (required for
> > > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.2 MHz
> > > + 1 = 26.0 MHz
> > > + 2 = 38.4 MHz
> > > + 3 = 52.0 MHz
> > > + 4 = 38.4 MHz, XTAL
> > > + 5 = 26.0 MHz, XTAL
> 
> This is just the omap refclock, right? If so, you can just pass the
> standard clock phandle. I know we don't yet have the DT clocks merged,
> but Tero just posted another revision of those.

This is an internal clock.  This clock is part of the module that
contains the WiLink chip.  It is not associated with the clocks in the
main board (OMAP).


> > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.200 MHz
> > > + 1 = 26.000 MHz
> > > + 2 = 38.400 MHz
> > > + 3 = 52.000 MHz
> > > + 4 = 16.368 MHz
> > > + 5 = 32.736 MHz
> > > + 6 = 16.800 MHz
> > > + 7 = 33.600 MHz
> 
> Where does this clock come from? Maybe this can be set based on the
> compatible value if it's completely internal?

This is also a completely internal clock.  My "compatible" values are
based on the WiLink chip itself, not in the module that contains the
chip.  There are several modules and they are the ones that specify the
clock frequencies.  This data I'm passing here is just to tell the
WiLink chip which frequencies the module uses.

My driver is for the WiLink chip itself, not to the module (in theory).
So I think having the WiLink values as bindings would be more generic
than having to specify values for each available module (eg.
"lsr-research,tiwi-ble") and mapping those values to specific
frequencies in the driver.

 
> > If this is okay for everyone, can I push this via my tree (which goes to
> > linux-wireless->net->linus)? I think it makes more sense to send the
> > documentation together with the patch that actually implements the DT
> > node parsing in the driver.
> 
> If we can use the standard bindings, it might be worth waiting until
> we have the DT clocks available as we have the pdata workaround merged
> anyways. That's because then we don't need to support the custom
> binding later on ;)

I looked into Tero's patches and I considered using the generic clock
bindings, but I think it doesn't make sense in this case.  The thing is
that the module is not providing the clocks to the main board.  Neither
is the WiLink chip consuming clocks from the main board.

I thought about specifying clock providers and consumers to be used only
by the module and WiLink chip, but I think it's overkill.  And we would
also have to find a way to prevent the main clock framework from trying
to handle them.

So, my conclusion was that, even though these *are* clocks, from the
main board's perspective they're just specifications of what the module
looks like.

Does this make sense?

--
Cheers,
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-26 Thread Tony Lindgren
* Luciano Coelho  [130625 12:43]:
> (oh crap, now *really* fixed the ARM mailing list address)
> 
> On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
> > Add device tree bindings documentation for the TI WiLink modules.
> > Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> > modules is supported.
> > 
> > Signed-off-by: Luciano Coelho 
> > ---
> > 
> > I created a new directory under net to contain wireless bindings 
> > documentation.
> > 
> > The actual implementation in the driver will follow separately.
> > 
> >  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
> > 
> >  1 file changed, 46 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
> > b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> > new file mode 100644
> > index 000..d8e8bfbb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> > @@ -0,0 +1,46 @@
> > +TI WiLink Wireless Modules Device Tree Bindings
> > +===
> > +
> > +The WiLink modules provide wireless connectivity, such as WLAN,
> > +Bluetooth, FM and NFC.
> > +
> > +There are several different modules available, which can be grouped by
> > +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
> > +currently supported with device tree.
> > +
> > +Currently, only the WLAN portion of the modules is supported with
> > +device tree.
> > +
> > +Required properties:
> > +
> > +
> > +- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
> > +- interrupt-parent: the interrupt controller
> > +- interrupts: out-of-band WLAN interrupt
> > +   See the interrupt controller's bindings documentation for
> > +   detailed definition.
> > +
> > +Optional properties:
> > +
> > +
> > +- refclock: the internal WLAN reference clock frequency (required for
> > +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.2 MHz
> > +   1 = 26.0 MHz
> > +   2 = 38.4 MHz
> > +   3 = 52.0 MHz
> > +   4 = 38.4 MHz, XTAL
> > +   5 = 26.0 MHz, XTAL

This is just the omap refclock, right? If so, you can just pass the
standard clock phandle. I know we don't yet have the DT clocks merged,
but Tero just posted another revision of those.

> > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.200 MHz
> > +   1 = 26.000 MHz
> > +   2 = 38.400 MHz
> > +   3 = 52.000 MHz
> > +   4 = 16.368 MHz
> > +   5 = 32.736 MHz
> > +   6 = 16.800 MHz
> > +   7 = 33.600 MHz

Where does this clock come from? Maybe this can be set based on the
compatible value if it's completely internal?
 
> If this is okay for everyone, can I push this via my tree (which goes to
> linux-wireless->net->linus)? I think it makes more sense to send the
> documentation together with the patch that actually implements the DT
> node parsing in the driver.

If we can use the standard bindings, it might be worth waiting until
we have the DT clocks available as we have the pdata workaround merged
anyways. That's because then we don't need to support the custom
binding later on ;)

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-26 Thread Tony Lindgren
* Luciano Coelho coe...@ti.com [130625 12:43]:
 (oh crap, now *really* fixed the ARM mailing list address)
 
 On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
  Add device tree bindings documentation for the TI WiLink modules.
  Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
  modules is supported.
  
  Signed-off-by: Luciano Coelho coe...@ti.com
  ---
  
  I created a new directory under net to contain wireless bindings 
  documentation.
  
  The actual implementation in the driver will follow separately.
  
   .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
  
   1 file changed, 46 insertions(+)
   create mode 100644 
  Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
  
  diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
  b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
  new file mode 100644
  index 000..d8e8bfbb
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
  @@ -0,0 +1,46 @@
  +TI WiLink Wireless Modules Device Tree Bindings
  +===
  +
  +The WiLink modules provide wireless connectivity, such as WLAN,
  +Bluetooth, FM and NFC.
  +
  +There are several different modules available, which can be grouped by
  +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
  +currently supported with device tree.
  +
  +Currently, only the WLAN portion of the modules is supported with
  +device tree.
  +
  +Required properties:
  +
  +
  +- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
  +- interrupt-parent: the interrupt controller
  +- interrupts: out-of-band WLAN interrupt
  +   See the interrupt controller's bindings documentation for
  +   detailed definition.
  +
  +Optional properties:
  +
  +
  +- refclock: the internal WLAN reference clock frequency (required for
  +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
  +  following:
  +   0 = 19.2 MHz
  +   1 = 26.0 MHz
  +   2 = 38.4 MHz
  +   3 = 52.0 MHz
  +   4 = 38.4 MHz, XTAL
  +   5 = 26.0 MHz, XTAL

This is just the omap refclock, right? If so, you can just pass the
standard clock phandle. I know we don't yet have the DT clocks merged,
but Tero just posted another revision of those.

  +- tcxoclock: the internal WLAN TCXO clock frequency (required for
  +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
  +  following:
  +   0 = 19.200 MHz
  +   1 = 26.000 MHz
  +   2 = 38.400 MHz
  +   3 = 52.000 MHz
  +   4 = 16.368 MHz
  +   5 = 32.736 MHz
  +   6 = 16.800 MHz
  +   7 = 33.600 MHz

Where does this clock come from? Maybe this can be set based on the
compatible value if it's completely internal?
 
 If this is okay for everyone, can I push this via my tree (which goes to
 linux-wireless-net-linus)? I think it makes more sense to send the
 documentation together with the patch that actually implements the DT
 node parsing in the driver.

If we can use the standard bindings, it might be worth waiting until
we have the DT clocks available as we have the pdata workaround merged
anyways. That's because then we don't need to support the custom
binding later on ;)

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-26 Thread Luciano Coelho
Hi Tony,

On Tue, 2013-06-25 at 23:24 -0700, Tony Lindgren wrote:
 * Luciano Coelho coe...@ti.com [130625 12:43]:
  On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
   Add device tree bindings documentation for the TI WiLink modules.
   Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
   modules is supported.
   
   Signed-off-by: Luciano Coelho coe...@ti.com
   ---

[...]

   +Optional properties:
   +
   +
   +- refclock: the internal WLAN reference clock frequency (required for
   +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
   +  following:
   + 0 = 19.2 MHz
   + 1 = 26.0 MHz
   + 2 = 38.4 MHz
   + 3 = 52.0 MHz
   + 4 = 38.4 MHz, XTAL
   + 5 = 26.0 MHz, XTAL
 
 This is just the omap refclock, right? If so, you can just pass the
 standard clock phandle. I know we don't yet have the DT clocks merged,
 but Tero just posted another revision of those.

This is an internal clock.  This clock is part of the module that
contains the WiLink chip.  It is not associated with the clocks in the
main board (OMAP).


   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
 
 Where does this clock come from? Maybe this can be set based on the
 compatible value if it's completely internal?

This is also a completely internal clock.  My compatible values are
based on the WiLink chip itself, not in the module that contains the
chip.  There are several modules and they are the ones that specify the
clock frequencies.  This data I'm passing here is just to tell the
WiLink chip which frequencies the module uses.

My driver is for the WiLink chip itself, not to the module (in theory).
So I think having the WiLink values as bindings would be more generic
than having to specify values for each available module (eg.
lsr-research,tiwi-ble) and mapping those values to specific
frequencies in the driver.

 
  If this is okay for everyone, can I push this via my tree (which goes to
  linux-wireless-net-linus)? I think it makes more sense to send the
  documentation together with the patch that actually implements the DT
  node parsing in the driver.
 
 If we can use the standard bindings, it might be worth waiting until
 we have the DT clocks available as we have the pdata workaround merged
 anyways. That's because then we don't need to support the custom
 binding later on ;)

I looked into Tero's patches and I considered using the generic clock
bindings, but I think it doesn't make sense in this case.  The thing is
that the module is not providing the clocks to the main board.  Neither
is the WiLink chip consuming clocks from the main board.

I thought about specifying clock providers and consumers to be used only
by the module and WiLink chip, but I think it's overkill.  And we would
also have to find a way to prevent the main clock framework from trying
to handle them.

So, my conclusion was that, even though these *are* clocks, from the
main board's perspective they're just specifications of what the module
looks like.

Does this make sense?

--
Cheers,
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-26 Thread Tony Lindgren
* Luciano Coelho coe...@ti.com [130626 01:19]:
 Hi Tony,
 
 On Tue, 2013-06-25 at 23:24 -0700, Tony Lindgren wrote:
  * Luciano Coelho coe...@ti.com [130625 12:43]:
   On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho coe...@ti.com
---
 
 [...]
 
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
  
  This is just the omap refclock, right? If so, you can just pass the
  standard clock phandle. I know we don't yet have the DT clocks merged,
  but Tero just posted another revision of those.
 
 This is an internal clock.  This clock is part of the module that
 contains the WiLink chip.  It is not associated with the clocks in the
 main board (OMAP).
 
 
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
  
  Where does this clock come from? Maybe this can be set based on the
  compatible value if it's completely internal?
 
 This is also a completely internal clock.  My compatible values are
 based on the WiLink chip itself, not in the module that contains the
 chip.  There are several modules and they are the ones that specify the
 clock frequencies.  This data I'm passing here is just to tell the
 WiLink chip which frequencies the module uses.
 
 My driver is for the WiLink chip itself, not to the module (in theory).
 So I think having the WiLink values as bindings would be more generic
 than having to specify values for each available module (eg.
 lsr-research,tiwi-ble) and mapping those values to specific
 frequencies in the driver.
 
  
   If this is okay for everyone, can I push this via my tree (which goes to
   linux-wireless-net-linus)? I think it makes more sense to send the
   documentation together with the patch that actually implements the DT
   node parsing in the driver.
  
  If we can use the standard bindings, it might be worth waiting until
  we have the DT clocks available as we have the pdata workaround merged
  anyways. That's because then we don't need to support the custom
  binding later on ;)
 
 I looked into Tero's patches and I considered using the generic clock
 bindings, but I think it doesn't make sense in this case.  The thing is
 that the module is not providing the clocks to the main board.  Neither
 is the WiLink chip consuming clocks from the main board.
 
 I thought about specifying clock providers and consumers to be used only
 by the module and WiLink chip, but I think it's overkill.  And we would
 also have to find a way to prevent the main clock framework from trying
 to handle them.
 
 So, my conclusion was that, even though these *are* clocks, from the
 main board's perspective they're just specifications of what the module
 looks like.
 
 Does this make sense?

OK yes, in that case looks fine to me:

Acked-by: Tony Lindgren t...@atomide.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
(oh crap, now *really* fixed the ARM mailing list address)

On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
> Add device tree bindings documentation for the TI WiLink modules.
> Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> modules is supported.
> 
> Signed-off-by: Luciano Coelho 
> ---
> 
> I created a new directory under net to contain wireless bindings 
> documentation.
> 
> The actual implementation in the driver will follow separately.
> 
>  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
> 
>  1 file changed, 46 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
> b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> new file mode 100644
> index 000..d8e8bfbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> @@ -0,0 +1,46 @@
> +TI WiLink Wireless Modules Device Tree Bindings
> +===
> +
> +The WiLink modules provide wireless connectivity, such as WLAN,
> +Bluetooth, FM and NFC.
> +
> +There are several different modules available, which can be grouped by
> +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
> +currently supported with device tree.
> +
> +Currently, only the WLAN portion of the modules is supported with
> +device tree.
> +
> +Required properties:
> +
> +
> +- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
> +- interrupt-parent: the interrupt controller
> +- interrupts: out-of-band WLAN interrupt
> + See the interrupt controller's bindings documentation for
> + detailed definition.
> +
> +Optional properties:
> +
> +
> +- refclock: the internal WLAN reference clock frequency (required for
> +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> +  following:
> + 0 = 19.2 MHz
> + 1 = 26.0 MHz
> + 2 = 38.4 MHz
> + 3 = 52.0 MHz
> + 4 = 38.4 MHz, XTAL
> + 5 = 26.0 MHz, XTAL
> +
> +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> +  following:
> + 0 = 19.200 MHz
> + 1 = 26.000 MHz
> + 2 = 38.400 MHz
> + 3 = 52.000 MHz
> + 4 = 16.368 MHz
> + 5 = 32.736 MHz
> + 6 = 16.800 MHz
> + 7 = 33.600 MHz

If this is okay for everyone, can I push this via my tree (which goes to
linux-wireless->net->linus)? I think it makes more sense to send the
documentation together with the patch that actually implements the DT
node parsing in the driver.

--
Luca.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
(fixed the ARM mailing list address)

On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
> Add device tree bindings documentation for the TI WiLink modules.
> Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> modules is supported.
> 
> Signed-off-by: Luciano Coelho 
> ---
> 
> I created a new directory under net to contain wireless bindings 
> documentation.
> 
> The actual implementation in the driver will follow separately.
> 
>  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
> 
>  1 file changed, 46 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
> b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> new file mode 100644
> index 000..d8e8bfbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> @@ -0,0 +1,46 @@
> +TI WiLink Wireless Modules Device Tree Bindings
> +===
> +
> +The WiLink modules provide wireless connectivity, such as WLAN,
> +Bluetooth, FM and NFC.
> +
> +There are several different modules available, which can be grouped by
> +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
> +currently supported with device tree.
> +
> +Currently, only the WLAN portion of the modules is supported with
> +device tree.
> +
> +Required properties:
> +
> +
> +- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
> +- interrupt-parent: the interrupt controller
> +- interrupts: out-of-band WLAN interrupt
> + See the interrupt controller's bindings documentation for
> + detailed definition.
> +
> +Optional properties:
> +
> +
> +- refclock: the internal WLAN reference clock frequency (required for
> +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> +  following:
> + 0 = 19.2 MHz
> + 1 = 26.0 MHz
> + 2 = 38.4 MHz
> + 3 = 52.0 MHz
> + 4 = 38.4 MHz, XTAL
> + 5 = 26.0 MHz, XTAL
> +
> +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> +  following:
> + 0 = 19.200 MHz
> + 1 = 26.000 MHz
> + 2 = 38.400 MHz
> + 3 = 52.000 MHz
> + 4 = 16.368 MHz
> + 5 = 32.736 MHz
> + 6 = 16.800 MHz
> + 7 = 33.600 MHz

If this is okay for everyone, can I push this via my tree (which goes to
linux-wireless->net->linus)? I think it makes more sense to send the
documentation together with the patch that actually implements the DT
node parsing in the driver.

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Felipe Balbi
On Tue, Jun 25, 2013 at 02:56:10PM +0300, Luciano Coelho wrote:
> On Tue, 2013-06-25 at 14:12 +0300, Felipe Balbi wrote:
> > On Tue, Jun 25, 2013 at 11:35:30AM +0300, Luciano Coelho wrote:
> > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > > +  following:
> > > + 0 = 19.200 MHz
> > > + 1 = 26.000 MHz
> > > + 2 = 38.400 MHz
> > > + 3 = 52.000 MHz
> > > + 4 = 16.368 MHz
> > > + 5 = 32.736 MHz
> > > + 6 = 16.800 MHz
> > > + 7 = 33.600 MHz
> > 
> > DTS files are pre-processed, so you could add defines in a header and
> > share the header between DTS and driver. Could help you having:
> > 
> > tcxoclock = WILINK_19_200MHz;
> > 
> > instead of
> > 
> > tcxoclock = 0;
> 
> I don't see any .dts file really doing this.  There are some imx*.dtsi
> files that include imx*.h files, but I don't see these headers being
> included in any source code file.
> 
> In fact, we already have all these values defined in
> include/linux/wl12xx.h, so it could be nice to reuse.  But the
> cross-directory includes would look "funny".  And I think it's a bit
> overkill.
> 
> These values are actually used by the firmware itself, not only the
> driver, so they are also platform independent and not related to the OS.

fair enough, then there's no chance they'll change all of a sudden.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
On Tue, 2013-06-25 at 14:12 +0300, Felipe Balbi wrote:
> On Tue, Jun 25, 2013 at 11:35:30AM +0300, Luciano Coelho wrote:
> > +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> > +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> > +  following:
> > +   0 = 19.200 MHz
> > +   1 = 26.000 MHz
> > +   2 = 38.400 MHz
> > +   3 = 52.000 MHz
> > +   4 = 16.368 MHz
> > +   5 = 32.736 MHz
> > +   6 = 16.800 MHz
> > +   7 = 33.600 MHz
> 
> DTS files are pre-processed, so you could add defines in a header and
> share the header between DTS and driver. Could help you having:
> 
> tcxoclock = WILINK_19_200MHz;
> 
> instead of
> 
> tcxoclock = 0;

I don't see any .dts file really doing this.  There are some imx*.dtsi
files that include imx*.h files, but I don't see these headers being
included in any source code file.

In fact, we already have all these values defined in
include/linux/wl12xx.h, so it could be nice to reuse.  But the
cross-directory includes would look "funny".  And I think it's a bit
overkill.

These values are actually used by the firmware itself, not only the
driver, so they are also platform independent and not related to the OS.

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Felipe Balbi
Hi,

On Tue, Jun 25, 2013 at 11:35:30AM +0300, Luciano Coelho wrote:
> Add device tree bindings documentation for the TI WiLink modules.
> Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
> modules is supported.
> 
> Signed-off-by: Luciano Coelho 
> ---
> 
> I created a new directory under net to contain wireless bindings 
> documentation.
> 
> The actual implementation in the driver will follow separately.
> 
>  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
> 
>  1 file changed, 46 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
> b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> new file mode 100644
> index 000..d8e8bfbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
> @@ -0,0 +1,46 @@
> +TI WiLink Wireless Modules Device Tree Bindings
> +===
> +
> +The WiLink modules provide wireless connectivity, such as WLAN,
> +Bluetooth, FM and NFC.
> +
> +There are several different modules available, which can be grouped by
> +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
> +currently supported with device tree.
> +
> +Currently, only the WLAN portion of the modules is supported with
> +device tree.
> +
> +Required properties:
> +
> +
> +- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
> +- interrupt-parent: the interrupt controller
> +- interrupts: out-of-band WLAN interrupt
> + See the interrupt controller's bindings documentation for
> + detailed definition.
> +
> +Optional properties:
> +
> +
> +- refclock: the internal WLAN reference clock frequency (required for
> +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
> +  following:
> + 0 = 19.2 MHz
> + 1 = 26.0 MHz
> + 2 = 38.4 MHz
> + 3 = 52.0 MHz
> + 4 = 38.4 MHz, XTAL
> + 5 = 26.0 MHz, XTAL
> +
> +- tcxoclock: the internal WLAN TCXO clock frequency (required for
> +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
> +  following:
> + 0 = 19.200 MHz
> + 1 = 26.000 MHz
> + 2 = 38.400 MHz
> + 3 = 52.000 MHz
> + 4 = 16.368 MHz
> + 5 = 32.736 MHz
> + 6 = 16.800 MHz
> + 7 = 33.600 MHz

DTS files are pre-processed, so you could add defines in a header and
share the header between DTS and driver. Could help you having:

tcxoclock = WILINK_19_200MHz;

instead of

tcxoclock = 0;

-- 
balbi


signature.asc
Description: Digital signature


[PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho 
---

I created a new directory under net to contain wireless bindings documentation.

The actual implementation in the driver will follow separately.

 .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
new file mode 100644
index 000..d8e8bfbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,46 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be "ti,wilink6", "ti,wilink7" or "ti,wilink8"
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+   See the interrupt controller's bindings documentation for
+   detailed definition.
+
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
Add device tree bindings documentation for the TI WiLink modules.
Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
modules is supported.

Signed-off-by: Luciano Coelho coe...@ti.com
---

I created a new directory under net to contain wireless bindings documentation.

The actual implementation in the driver will follow separately.

 .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt

diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
new file mode 100644
index 000..d8e8bfbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
@@ -0,0 +1,46 @@
+TI WiLink Wireless Modules Device Tree Bindings
+===
+
+The WiLink modules provide wireless connectivity, such as WLAN,
+Bluetooth, FM and NFC.
+
+There are several different modules available, which can be grouped by
+their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
+currently supported with device tree.
+
+Currently, only the WLAN portion of the modules is supported with
+device tree.
+
+Required properties:
+
+
+- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
+- interrupt-parent: the interrupt controller
+- interrupts: out-of-band WLAN interrupt
+   See the interrupt controller's bindings documentation for
+   detailed definition.
+
+Optional properties:
+
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+   0 = 19.2 MHz
+   1 = 26.0 MHz
+   2 = 38.4 MHz
+   3 = 52.0 MHz
+   4 = 38.4 MHz, XTAL
+   5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+   0 = 19.200 MHz
+   1 = 26.000 MHz
+   2 = 38.400 MHz
+   3 = 52.000 MHz
+   4 = 16.368 MHz
+   5 = 32.736 MHz
+   6 = 16.800 MHz
+   7 = 33.600 MHz
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Felipe Balbi
Hi,

On Tue, Jun 25, 2013 at 11:35:30AM +0300, Luciano Coelho wrote:
 Add device tree bindings documentation for the TI WiLink modules.
 Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
 modules is supported.
 
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---
 
 I created a new directory under net to contain wireless bindings 
 documentation.
 
 The actual implementation in the driver will follow separately.
 
  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
 
  1 file changed, 46 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 
 diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
 b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 new file mode 100644
 index 000..d8e8bfbb
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 @@ -0,0 +1,46 @@
 +TI WiLink Wireless Modules Device Tree Bindings
 +===
 +
 +The WiLink modules provide wireless connectivity, such as WLAN,
 +Bluetooth, FM and NFC.
 +
 +There are several different modules available, which can be grouped by
 +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
 +currently supported with device tree.
 +
 +Currently, only the WLAN portion of the modules is supported with
 +device tree.
 +
 +Required properties:
 +
 +
 +- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
 +- interrupt-parent: the interrupt controller
 +- interrupts: out-of-band WLAN interrupt
 + See the interrupt controller's bindings documentation for
 + detailed definition.
 +
 +Optional properties:
 +
 +
 +- refclock: the internal WLAN reference clock frequency (required for
 +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
 +  following:
 + 0 = 19.2 MHz
 + 1 = 26.0 MHz
 + 2 = 38.4 MHz
 + 3 = 52.0 MHz
 + 4 = 38.4 MHz, XTAL
 + 5 = 26.0 MHz, XTAL
 +
 +- tcxoclock: the internal WLAN TCXO clock frequency (required for
 +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
 +  following:
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz

DTS files are pre-processed, so you could add defines in a header and
share the header between DTS and driver. Could help you having:

tcxoclock = WILINK_19_200MHz;

instead of

tcxoclock = 0;

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
On Tue, 2013-06-25 at 14:12 +0300, Felipe Balbi wrote:
 On Tue, Jun 25, 2013 at 11:35:30AM +0300, Luciano Coelho wrote:
  +- tcxoclock: the internal WLAN TCXO clock frequency (required for
  +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
  +  following:
  +   0 = 19.200 MHz
  +   1 = 26.000 MHz
  +   2 = 38.400 MHz
  +   3 = 52.000 MHz
  +   4 = 16.368 MHz
  +   5 = 32.736 MHz
  +   6 = 16.800 MHz
  +   7 = 33.600 MHz
 
 DTS files are pre-processed, so you could add defines in a header and
 share the header between DTS and driver. Could help you having:
 
 tcxoclock = WILINK_19_200MHz;
 
 instead of
 
 tcxoclock = 0;

I don't see any .dts file really doing this.  There are some imx*.dtsi
files that include imx*.h files, but I don't see these headers being
included in any source code file.

In fact, we already have all these values defined in
include/linux/wl12xx.h, so it could be nice to reuse.  But the
cross-directory includes would look funny.  And I think it's a bit
overkill.

These values are actually used by the firmware itself, not only the
driver, so they are also platform independent and not related to the OS.

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Felipe Balbi
On Tue, Jun 25, 2013 at 02:56:10PM +0300, Luciano Coelho wrote:
 On Tue, 2013-06-25 at 14:12 +0300, Felipe Balbi wrote:
  On Tue, Jun 25, 2013 at 11:35:30AM +0300, Luciano Coelho wrote:
   +- tcxoclock: the internal WLAN TCXO clock frequency (required for
   +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
   +  following:
   + 0 = 19.200 MHz
   + 1 = 26.000 MHz
   + 2 = 38.400 MHz
   + 3 = 52.000 MHz
   + 4 = 16.368 MHz
   + 5 = 32.736 MHz
   + 6 = 16.800 MHz
   + 7 = 33.600 MHz
  
  DTS files are pre-processed, so you could add defines in a header and
  share the header between DTS and driver. Could help you having:
  
  tcxoclock = WILINK_19_200MHz;
  
  instead of
  
  tcxoclock = 0;
 
 I don't see any .dts file really doing this.  There are some imx*.dtsi
 files that include imx*.h files, but I don't see these headers being
 included in any source code file.
 
 In fact, we already have all these values defined in
 include/linux/wl12xx.h, so it could be nice to reuse.  But the
 cross-directory includes would look funny.  And I think it's a bit
 overkill.
 
 These values are actually used by the firmware itself, not only the
 driver, so they are also platform independent and not related to the OS.

fair enough, then there's no chance they'll change all of a sudden.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
(fixed the ARM mailing list address)

On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
 Add device tree bindings documentation for the TI WiLink modules.
 Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
 modules is supported.
 
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---
 
 I created a new directory under net to contain wireless bindings 
 documentation.
 
 The actual implementation in the driver will follow separately.
 
  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
 
  1 file changed, 46 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 
 diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
 b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 new file mode 100644
 index 000..d8e8bfbb
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 @@ -0,0 +1,46 @@
 +TI WiLink Wireless Modules Device Tree Bindings
 +===
 +
 +The WiLink modules provide wireless connectivity, such as WLAN,
 +Bluetooth, FM and NFC.
 +
 +There are several different modules available, which can be grouped by
 +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
 +currently supported with device tree.
 +
 +Currently, only the WLAN portion of the modules is supported with
 +device tree.
 +
 +Required properties:
 +
 +
 +- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
 +- interrupt-parent: the interrupt controller
 +- interrupts: out-of-band WLAN interrupt
 + See the interrupt controller's bindings documentation for
 + detailed definition.
 +
 +Optional properties:
 +
 +
 +- refclock: the internal WLAN reference clock frequency (required for
 +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
 +  following:
 + 0 = 19.2 MHz
 + 1 = 26.0 MHz
 + 2 = 38.4 MHz
 + 3 = 52.0 MHz
 + 4 = 38.4 MHz, XTAL
 + 5 = 26.0 MHz, XTAL
 +
 +- tcxoclock: the internal WLAN TCXO clock frequency (required for
 +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
 +  following:
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz

If this is okay for everyone, can I push this via my tree (which goes to
linux-wireless-net-linus)? I think it makes more sense to send the
documentation together with the patch that actually implements the DT
node parsing in the driver.

--
Luca.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: dt: bindings: TI WiLink modules

2013-06-25 Thread Luciano Coelho
(oh crap, now *really* fixed the ARM mailing list address)

On Tue, 2013-06-25 at 11:35 +0300, Luciano Coelho wrote:
 Add device tree bindings documentation for the TI WiLink modules.
 Currently only the WLAN part of the WiLink6, WiLink7 and WiLink8
 modules is supported.
 
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---
 
 I created a new directory under net to contain wireless bindings 
 documentation.
 
 The actual implementation in the driver will follow separately.
 
  .../devicetree/bindings/net/wireless/ti-wilink.txt |   46 
 
  1 file changed, 46 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 
 diff --git a/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt 
 b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 new file mode 100644
 index 000..d8e8bfbb
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/net/wireless/ti-wilink.txt
 @@ -0,0 +1,46 @@
 +TI WiLink Wireless Modules Device Tree Bindings
 +===
 +
 +The WiLink modules provide wireless connectivity, such as WLAN,
 +Bluetooth, FM and NFC.
 +
 +There are several different modules available, which can be grouped by
 +their generation: WiLink6, WiLink7 and WiLink8.  WiLink4 is not
 +currently supported with device tree.
 +
 +Currently, only the WLAN portion of the modules is supported with
 +device tree.
 +
 +Required properties:
 +
 +
 +- compatible: should be ti,wilink6, ti,wilink7 or ti,wilink8
 +- interrupt-parent: the interrupt controller
 +- interrupts: out-of-band WLAN interrupt
 + See the interrupt controller's bindings documentation for
 + detailed definition.
 +
 +Optional properties:
 +
 +
 +- refclock: the internal WLAN reference clock frequency (required for
 +  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
 +  following:
 + 0 = 19.2 MHz
 + 1 = 26.0 MHz
 + 2 = 38.4 MHz
 + 3 = 52.0 MHz
 + 4 = 38.4 MHz, XTAL
 + 5 = 26.0 MHz, XTAL
 +
 +- tcxoclock: the internal WLAN TCXO clock frequency (required for
 +  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
 +  following:
 + 0 = 19.200 MHz
 + 1 = 26.000 MHz
 + 2 = 38.400 MHz
 + 3 = 52.000 MHz
 + 4 = 16.368 MHz
 + 5 = 32.736 MHz
 + 6 = 16.800 MHz
 + 7 = 33.600 MHz

If this is okay for everyone, can I push this via my tree (which goes to
linux-wireless-net-linus)? I think it makes more sense to send the
documentation together with the patch that actually implements the DT
node parsing in the driver.

--
Luca.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/