Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-09 Thread Stefan Agner
On 2015-03-09 10:58, Sascha Hauer wrote:
> On Fri, Mar 06, 2015 at 02:31:51PM +0100, Stefan Agner wrote:
>> On 2015-03-06 07:15, Sascha Hauer wrote:
>> > Hi Stefan,
>> >
>> > On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>> >> +
>> >> +static int vf610_nfc_probe_dt(struct device *dev, struct 
>> >> vf610_nfc_config *cfg)
>> >> +{
>> >> + struct device_node *np = dev->of_node;
>> >> + int buswidth;
>> >> + u32 clkrate;
>> >> +
>> >> + if (!np)
>> >> + return 1;
>> >> +
>> >> + cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>> >> +
>> >> + if (!of_property_read_u32(np, "clock-frequency", ))
>> >> + cfg->clkrate = clkrate;
>> >
>> > Normally the clock-frequency property tells the driver at which
>> > frequency the device actually is running, not to tell the driver at
>> > which frequency the device *should* run. It's strange to use the value
>> > of the clock-frequency property as input to clk_set_rate(). Maybe the
>> > assigned clock binding is more appropriate here, see
>> > Documentation/devicetree/bindings/clock/clock-bindings.txt.
>>
>> What we try to do here is to specify the hardware limitations. There
>> seem to be some hardware restrictions when it comes to clock
>> frequencies. There has been a rather long discussion over at Freescales
>> community about it:
>> https://community.freescale.com/thread/317074
>>
>> Not sure if this is the right way to specify the supported frequencies,
>> or should we create a custom property for this, something like
>> fsl,max-nfc-frequency = <3300>?
> 
> What's wrong with the assigned clock binding? All you have to do is to
> add it to the device node. The rest will be done from the generic clock
> framework with no additional driver code.

Ah yes, assigned clock bindings seems like a match. So, the NFC node
would look something like

...
assigned-clocks = < VF610_CLK_NFC>;
assigned-clock-rates = <3300>;
...

Will try to use that, thx.

--
Stefan




--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-09 Thread Sascha Hauer
On Fri, Mar 06, 2015 at 02:31:51PM +0100, Stefan Agner wrote:
> On 2015-03-06 07:15, Sascha Hauer wrote:
> > Hi Stefan,
> > 
> > On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
> >> +
> >> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config 
> >> *cfg)
> >> +{
> >> +  struct device_node *np = dev->of_node;
> >> +  int buswidth;
> >> +  u32 clkrate;
> >> +
> >> +  if (!np)
> >> +  return 1;
> >> +
> >> +  cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
> >> +
> >> +  if (!of_property_read_u32(np, "clock-frequency", ))
> >> +  cfg->clkrate = clkrate;
> > 
> > Normally the clock-frequency property tells the driver at which
> > frequency the device actually is running, not to tell the driver at
> > which frequency the device *should* run. It's strange to use the value
> > of the clock-frequency property as input to clk_set_rate(). Maybe the
> > assigned clock binding is more appropriate here, see
> > Documentation/devicetree/bindings/clock/clock-bindings.txt.
> 
> What we try to do here is to specify the hardware limitations. There
> seem to be some hardware restrictions when it comes to clock
> frequencies. There has been a rather long discussion over at Freescales
> community about it:
> https://community.freescale.com/thread/317074
> 
> Not sure if this is the right way to specify the supported frequencies,
> or should we create a custom property for this, something like
> fsl,max-nfc-frequency = <3300>?

What's wrong with the assigned clock binding? All you have to do is to
add it to the device node. The rest will be done from the generic clock
framework with no additional driver code.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-09 Thread Stefan Agner
On 2015-03-06 16:32, Bill Pringlemeir wrote:
> On  6 Mar 2015, ste...@agner.ch wrote:
> 
>> On 2015-03-06 07:15, Sascha Hauer wrote:
>>> Hi Stefan,
> 
>>> On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
 +
 +static int vf610_nfc_probe_dt(struct device *dev, struct
 vf610_nfc_config *cfg)
 +{
 +  struct device_node *np = dev->of_node;
 +  int buswidth;
 +  u32 clkrate;
 +
 +  if (!np)
 +  return 1;
 +
 +  cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
 +
 +  if (!of_property_read_u32(np, "clock-frequency", ))
 +  cfg->clkrate = clkrate;
> 
>>> Normally the clock-frequency property tells the driver at which
>>> frequency the device actually is running, not to tell the driver at
>>> which frequency the device *should* run. It's strange to use the
>>> value of the clock-frequency property as input to
>>> clk_set_rate(). Maybe the assigned clock binding is more appropriate
>>> here, see Documentation/devicetree/bindings/clock/clock-bindings.txt.
> 
>> What we try to do here is to specify the hardware limitations. There
>> seem to be some hardware restrictions when it comes to clock
>> frequencies. There has been a rather long discussion over at
>> Freescales community about it:
>> https://community.freescale.com/thread/317074
> 
>> Not sure if this is the right way to specify the supported
>> frequencies, or should we create a custom property for this, something
>> like fsl,max-nfc-frequency = <3300>?
> 
> On most SOC's with this controller, the input clock to the controller
> affects the NAND flash timing and other factors; so you will want to set
> it based on the board/NAND stuffed.  The clock is for synchronous logic
> in the controller and affects many properties.  
> 
> I guess Sascha's point is, the board's DT should just have some
> '_clk' node and not have this part of the driver?  Either way works.
> However, this clock is very important to get the driver to function.  It
> seem better for a user/porter to have this info in the node.  I guess
> you need to be trained to look at every node in the sub-tree for a
> device.  I think the other way might be better or a sub-system
> maintainer.  I looked at 'i2c' and other node which have a
> 'clock-frequency' parameter. 
> 
> In the Documentation/devicetree/bindings/clock/clock-bindings.txt,
> 
> uart@a000 {
> compatible = "fsl,imx-uart";
> reg = <0xa000 0x1000>;
> interrupts = <33>;
> clocks = < 0>, < 1>;
> clock-names = "baud", "register";
> };
> 
> Here, this uart clock may affect the maximum baud rate supported by the
> device.  For this controller (vf610_nfc), the clock is like setting the
> 'baud rate'; it affect the NAND memory cycles.  There is not really any
> 'wait state' type logic in the controller register set that would allow
> the driver to work with a 'given clock' rate.  For certain a board
> should set this clock for the NAND chips they wish to support.
> 
> What would the board file look like to use clock node?
> 
> [generic]
> nfc: nand@400e {
> compatible = "fsl,vf610-nfc";
> reg = <0x400e 0x4000>;
> clocks = < VF610_CLK_NFC>;
> clock-names = "nfc_clk";
> status = "disabled";
> };
> 
> [board]
> 
>  {
> nand-bus-width = <16>;
> nand-ecc-mode = "hw";
> nand-on-flash-bbt;
> nand-ecc-strength = <24>;
> nand-ecc-step-size = <2048>;
> pinctrl-names = "default";
> pinctrl-0 = <_nfc_1>;
> status = "okay";
> _clk { frequency = <3300>}  /* Like this? */

There is a property called "clock-frequency", but this is really for
static clocks (e.g. external oscillators). I don't think this is the
right approach.

> };
> 
> I don't know how to do the 'Like this?' part.  I don't think the
> 'clock-bindings.txt' explains it.  I see this is better as the the
> driver needs no 'clock handling' code.  It is definitely a little more
> obtuse for the users.

There are drivers which use the clock-frequency property to actually
specify maximum operating frequencies:

Documentation/devicetree/bindings/i2c/i2c-efm32.txt: - clock-frequency :
maximal I2C bus clock frequency in Hz.
Documentation/devicetree/bindings/media/samsung-fimc.txt:-
clock-frequency: maximum FIMC local clock (LCLK) frequency;

In the MMC subsystem there is a property called max-frequency, the SPI
subsystem uses the property spi-max-frequency to specify maximum
operating frequencies of SPI slaves.

How about just max-frequency? Or with vendor prefix, e.g.
fsl,max-frequency...?

> 
> [snip]
> 
>>> Does this driver work without device tree or not? Currently the
>>> driver bails out when device tree support is enabled but no device
>>> node is given. When device tree support is disabled in the kernel
>>> though the driver happily continues here.
> 
>> Hm, I never tried using this 

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-09 Thread Stefan Agner
On 2015-03-09 10:58, Sascha Hauer wrote:
 On Fri, Mar 06, 2015 at 02:31:51PM +0100, Stefan Agner wrote:
 On 2015-03-06 07:15, Sascha Hauer wrote:
  Hi Stefan,
 
  On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
  +
  +static int vf610_nfc_probe_dt(struct device *dev, struct 
  vf610_nfc_config *cfg)
  +{
  + struct device_node *np = dev-of_node;
  + int buswidth;
  + u32 clkrate;
  +
  + if (!np)
  + return 1;
  +
  + cfg-flash_bbt = of_get_nand_on_flash_bbt(np);
  +
  + if (!of_property_read_u32(np, clock-frequency, clkrate))
  + cfg-clkrate = clkrate;
 
  Normally the clock-frequency property tells the driver at which
  frequency the device actually is running, not to tell the driver at
  which frequency the device *should* run. It's strange to use the value
  of the clock-frequency property as input to clk_set_rate(). Maybe the
  assigned clock binding is more appropriate here, see
  Documentation/devicetree/bindings/clock/clock-bindings.txt.

 What we try to do here is to specify the hardware limitations. There
 seem to be some hardware restrictions when it comes to clock
 frequencies. There has been a rather long discussion over at Freescales
 community about it:
 https://community.freescale.com/thread/317074

 Not sure if this is the right way to specify the supported frequencies,
 or should we create a custom property for this, something like
 fsl,max-nfc-frequency = 3300?
 
 What's wrong with the assigned clock binding? All you have to do is to
 add it to the device node. The rest will be done from the generic clock
 framework with no additional driver code.

Ah yes, assigned clock bindings seems like a match. So, the NFC node
would look something like

...
assigned-clocks = clks VF610_CLK_NFC;
assigned-clock-rates = 3300;
...

Will try to use that, thx.

--
Stefan




--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-09 Thread Stefan Agner
On 2015-03-06 16:32, Bill Pringlemeir wrote:
 On  6 Mar 2015, ste...@agner.ch wrote:
 
 On 2015-03-06 07:15, Sascha Hauer wrote:
 Hi Stefan,
 
 On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
 +
 +static int vf610_nfc_probe_dt(struct device *dev, struct
 vf610_nfc_config *cfg)
 +{
 +  struct device_node *np = dev-of_node;
 +  int buswidth;
 +  u32 clkrate;
 +
 +  if (!np)
 +  return 1;
 +
 +  cfg-flash_bbt = of_get_nand_on_flash_bbt(np);
 +
 +  if (!of_property_read_u32(np, clock-frequency, clkrate))
 +  cfg-clkrate = clkrate;
 
 Normally the clock-frequency property tells the driver at which
 frequency the device actually is running, not to tell the driver at
 which frequency the device *should* run. It's strange to use the
 value of the clock-frequency property as input to
 clk_set_rate(). Maybe the assigned clock binding is more appropriate
 here, see Documentation/devicetree/bindings/clock/clock-bindings.txt.
 
 What we try to do here is to specify the hardware limitations. There
 seem to be some hardware restrictions when it comes to clock
 frequencies. There has been a rather long discussion over at
 Freescales community about it:
 https://community.freescale.com/thread/317074
 
 Not sure if this is the right way to specify the supported
 frequencies, or should we create a custom property for this, something
 like fsl,max-nfc-frequency = 3300?
 
 On most SOC's with this controller, the input clock to the controller
 affects the NAND flash timing and other factors; so you will want to set
 it based on the board/NAND stuffed.  The clock is for synchronous logic
 in the controller and affects many properties.  
 
 I guess Sascha's point is, the board's DT should just have some
 'nfc_clk' node and not have this part of the driver?  Either way works.
 However, this clock is very important to get the driver to function.  It
 seem better for a user/porter to have this info in the node.  I guess
 you need to be trained to look at every node in the sub-tree for a
 device.  I think the other way might be better or a sub-system
 maintainer.  I looked at 'i2c' and other node which have a
 'clock-frequency' parameter. 
 
 In the Documentation/devicetree/bindings/clock/clock-bindings.txt,
 
 uart@a000 {
 compatible = fsl,imx-uart;
 reg = 0xa000 0x1000;
 interrupts = 33;
 clocks = osc 0, pll 1;
 clock-names = baud, register;
 };
 
 Here, this uart clock may affect the maximum baud rate supported by the
 device.  For this controller (vf610_nfc), the clock is like setting the
 'baud rate'; it affect the NAND memory cycles.  There is not really any
 'wait state' type logic in the controller register set that would allow
 the driver to work with a 'given clock' rate.  For certain a board
 should set this clock for the NAND chips they wish to support.
 
 What would the board file look like to use clock node?
 
 [generic]
 nfc: nand@400e {
 compatible = fsl,vf610-nfc;
 reg = 0x400e 0x4000;
 clocks = clks VF610_CLK_NFC;
 clock-names = nfc_clk;
 status = disabled;
 };
 
 [board]
 
 nfc {
 nand-bus-width = 16;
 nand-ecc-mode = hw;
 nand-on-flash-bbt;
 nand-ecc-strength = 24;
 nand-ecc-step-size = 2048;
 pinctrl-names = default;
 pinctrl-0 = pinctrl_nfc_1;
 status = okay;
 nfc_clk { frequency = 3300}  /* Like this? */

There is a property called clock-frequency, but this is really for
static clocks (e.g. external oscillators). I don't think this is the
right approach.

 };
 
 I don't know how to do the 'Like this?' part.  I don't think the
 'clock-bindings.txt' explains it.  I see this is better as the the
 driver needs no 'clock handling' code.  It is definitely a little more
 obtuse for the users.

There are drivers which use the clock-frequency property to actually
specify maximum operating frequencies:

Documentation/devicetree/bindings/i2c/i2c-efm32.txt: - clock-frequency :
maximal I2C bus clock frequency in Hz.
Documentation/devicetree/bindings/media/samsung-fimc.txt:-
clock-frequency: maximum FIMC local clock (LCLK) frequency;

In the MMC subsystem there is a property called max-frequency, the SPI
subsystem uses the property spi-max-frequency to specify maximum
operating frequencies of SPI slaves.

How about just max-frequency? Or with vendor prefix, e.g.
fsl,max-frequency...?

 
 [snip]
 
 Does this driver work without device tree or not? Currently the
 driver bails out when device tree support is enabled but no device
 node is given. When device tree support is disabled in the kernel
 though the driver happily continues here.
 
 Hm, I never tried using this Driver without DT.
 
 [snip]
 
 I also didn't test this.  The driver was ported from Linux versions
 where DT did not exist.  It is used in some OpenWRT/68k/coldfire
 (patched) kernels and I wanted it to 

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-09 Thread Sascha Hauer
On Fri, Mar 06, 2015 at 02:31:51PM +0100, Stefan Agner wrote:
 On 2015-03-06 07:15, Sascha Hauer wrote:
  Hi Stefan,
  
  On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
  +
  +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config 
  *cfg)
  +{
  +  struct device_node *np = dev-of_node;
  +  int buswidth;
  +  u32 clkrate;
  +
  +  if (!np)
  +  return 1;
  +
  +  cfg-flash_bbt = of_get_nand_on_flash_bbt(np);
  +
  +  if (!of_property_read_u32(np, clock-frequency, clkrate))
  +  cfg-clkrate = clkrate;
  
  Normally the clock-frequency property tells the driver at which
  frequency the device actually is running, not to tell the driver at
  which frequency the device *should* run. It's strange to use the value
  of the clock-frequency property as input to clk_set_rate(). Maybe the
  assigned clock binding is more appropriate here, see
  Documentation/devicetree/bindings/clock/clock-bindings.txt.
 
 What we try to do here is to specify the hardware limitations. There
 seem to be some hardware restrictions when it comes to clock
 frequencies. There has been a rather long discussion over at Freescales
 community about it:
 https://community.freescale.com/thread/317074
 
 Not sure if this is the right way to specify the supported frequencies,
 or should we create a custom property for this, something like
 fsl,max-nfc-frequency = 3300?

What's wrong with the assigned clock binding? All you have to do is to
add it to the device node. The rest will be done from the generic clock
framework with no additional driver code.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-06 Thread Bill Pringlemeir
On  6 Mar 2015, ste...@agner.ch wrote:

> On 2015-03-06 07:15, Sascha Hauer wrote:
>> Hi Stefan,

>> On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>>> +
>>> +static int vf610_nfc_probe_dt(struct device *dev, struct
>>> vf610_nfc_config *cfg)
>>> +{
>>> +   struct device_node *np = dev->of_node;
>>> +   int buswidth;
>>> +   u32 clkrate;
>>> +
>>> +   if (!np)
>>> +   return 1;
>>> +
>>> +   cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>>> +
>>> +   if (!of_property_read_u32(np, "clock-frequency", ))
>>> +   cfg->clkrate = clkrate;

>> Normally the clock-frequency property tells the driver at which
>> frequency the device actually is running, not to tell the driver at
>> which frequency the device *should* run. It's strange to use the
>> value of the clock-frequency property as input to
>> clk_set_rate(). Maybe the assigned clock binding is more appropriate
>> here, see Documentation/devicetree/bindings/clock/clock-bindings.txt.

> What we try to do here is to specify the hardware limitations. There
> seem to be some hardware restrictions when it comes to clock
> frequencies. There has been a rather long discussion over at
> Freescales community about it:
> https://community.freescale.com/thread/317074

> Not sure if this is the right way to specify the supported
> frequencies, or should we create a custom property for this, something
> like fsl,max-nfc-frequency = <3300>?

On most SOC's with this controller, the input clock to the controller
affects the NAND flash timing and other factors; so you will want to set
it based on the board/NAND stuffed.  The clock is for synchronous logic
in the controller and affects many properties.  

I guess Sascha's point is, the board's DT should just have some
'_clk' node and not have this part of the driver?  Either way works.
However, this clock is very important to get the driver to function.  It
seem better for a user/porter to have this info in the node.  I guess
you need to be trained to look at every node in the sub-tree for a
device.  I think the other way might be better or a sub-system
maintainer.  I looked at 'i2c' and other node which have a
'clock-frequency' parameter. 

In the Documentation/devicetree/bindings/clock/clock-bindings.txt,

uart@a000 {
compatible = "fsl,imx-uart";
reg = <0xa000 0x1000>;
interrupts = <33>;
clocks = < 0>, < 1>;
clock-names = "baud", "register";
};

Here, this uart clock may affect the maximum baud rate supported by the
device.  For this controller (vf610_nfc), the clock is like setting the
'baud rate'; it affect the NAND memory cycles.  There is not really any
'wait state' type logic in the controller register set that would allow
the driver to work with a 'given clock' rate.  For certain a board
should set this clock for the NAND chips they wish to support.

What would the board file look like to use clock node?

[generic]
nfc: nand@400e {
compatible = "fsl,vf610-nfc";
reg = <0x400e 0x4000>;
clocks = < VF610_CLK_NFC>;
clock-names = "nfc_clk";
status = "disabled";
};

[board]

 {
nand-bus-width = <16>;
nand-ecc-mode = "hw";
nand-on-flash-bbt;
nand-ecc-strength = <24>;
nand-ecc-step-size = <2048>;
pinctrl-names = "default";
pinctrl-0 = <_nfc_1>;
status = "okay";
_clk { frequency = <3300>}  /* Like this? */
};

I don't know how to do the 'Like this?' part.  I don't think the
'clock-bindings.txt' explains it.  I see this is better as the the
driver needs no 'clock handling' code.  It is definitely a little more
obtuse for the users.

[snip]

>> Does this driver work without device tree or not? Currently the
>> driver bails out when device tree support is enabled but no device
>> node is given. When device tree support is disabled in the kernel
>> though the driver happily continues here.

> Hm, I never tried using this Driver without DT. 

[snip]

I also didn't test this.  The driver was ported from Linux versions
where DT did not exist.  It is used in some OpenWRT/68k/coldfire
(patched) kernels and I wanted it to be useful for them.  However, we
could probably remove the 'platform support'.  Other people are using
this on PPC platforms and they will also have dt/of.

Currently the platform control has no way to 'pass data', so the driver
only works with whatever defaults it has (or that is my belief).  For
instance, those OpenWRT kernels have a 'machine file' which will set the
'clock-frequency' and other parameters.  We could remove the platform
support completely if it is misleading.  I guess the KConfig would need
a 'depends on CONFIG_OF'.

Thanks for the review,
Bill Pringlemeir.
--
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  

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-06 Thread Stefan Agner
On 2015-03-06 07:15, Sascha Hauer wrote:
> Hi Stefan,
> 
> On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
>> +
>> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config 
>> *cfg)
>> +{
>> +struct device_node *np = dev->of_node;
>> +int buswidth;
>> +u32 clkrate;
>> +
>> +if (!np)
>> +return 1;
>> +
>> +cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
>> +
>> +if (!of_property_read_u32(np, "clock-frequency", ))
>> +cfg->clkrate = clkrate;
> 
> Normally the clock-frequency property tells the driver at which
> frequency the device actually is running, not to tell the driver at
> which frequency the device *should* run. It's strange to use the value
> of the clock-frequency property as input to clk_set_rate(). Maybe the
> assigned clock binding is more appropriate here, see
> Documentation/devicetree/bindings/clock/clock-bindings.txt.

What we try to do here is to specify the hardware limitations. There
seem to be some hardware restrictions when it comes to clock
frequencies. There has been a rather long discussion over at Freescales
community about it:
https://community.freescale.com/thread/317074

Not sure if this is the right way to specify the supported frequencies,
or should we create a custom property for this, something like
fsl,max-nfc-frequency = <3300>?


> 
> BTW the above can easier be written as:
> 
>   of_property_read_u32(np, "clock-frequency", >clkrate);
> 
> No return value checking necessary.
> 
>> +static int vf610_nfc_probe(struct platform_device *pdev)
>> +{
>> +struct vf610_nfc *nfc;
>> +struct resource *res;
>> +struct mtd_info *mtd;
>> +struct nand_chip *chip;
>> +struct vf610_nfc_config *cfg;
>> +int err = 0;
>> +int page_sz;
>> +int irq;
>> +
>> +nfc = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
>> +if (!nfc)
>> +return -ENOMEM;
>> +
>> +nfc->cfg = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
>> +if (!nfc->cfg)
>> +return -ENOMEM;
>> +cfg = nfc->cfg;
> 
> Why is nfc->cfg allocated separately instead of embedding it into struct
> vf610_nfc? Is this some platform_data leftover you can remove now?
> 

Yeah, looks like a platform_data leftover, will change that.

>> +
>> +nfc->dev = >dev;
>> +nfc->page = -1;
>> +mtd = >mtd;
>> +chip = >chip;
>> +
>> +mtd->priv = chip;
>> +mtd->owner = THIS_MODULE;
>> +mtd->dev.parent = nfc->dev;
>> +mtd->name = DRV_NAME;
>> +
>> +err = vf610_nfc_probe_dt(nfc->dev, cfg);
>> +if (err)
>> +return -ENODEV;
> 
> Does this driver work without device tree or not? Currently the driver
> bails out when device tree support is enabled but no device node is
> given. When device tree support is disabled in the kernel though the
> driver happily continues here.
> 

Hm, I never tried using this Driver without DT. I guess in practice, we
won't get here in a DT enabled kernel anyway: If there is no device tree
node with one of the drivers compatibility ids, the probe function won't
be called.

Theoretically, the IP supported by this driver is part of two other
SoC's which are supported by Linux: PowerPC MPC5125 (DT) and a ColdFire
MCF54418 (non-DT). So this driver might be used on a ColdFire SoC using
the platform support. Currently, one would just not have the ability to
set the hardware ECC options... I'm a bit reluctant to implement full
platform support without being able to properly test it. If anybody is
interested in it, one could easily extend the driver...

Put Geert in CC...

--
Stefan

--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-06 Thread Stefan Agner
On 2015-03-06 07:15, Sascha Hauer wrote:
 Hi Stefan,
 
 On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
 +
 +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config 
 *cfg)
 +{
 +struct device_node *np = dev-of_node;
 +int buswidth;
 +u32 clkrate;
 +
 +if (!np)
 +return 1;
 +
 +cfg-flash_bbt = of_get_nand_on_flash_bbt(np);
 +
 +if (!of_property_read_u32(np, clock-frequency, clkrate))
 +cfg-clkrate = clkrate;
 
 Normally the clock-frequency property tells the driver at which
 frequency the device actually is running, not to tell the driver at
 which frequency the device *should* run. It's strange to use the value
 of the clock-frequency property as input to clk_set_rate(). Maybe the
 assigned clock binding is more appropriate here, see
 Documentation/devicetree/bindings/clock/clock-bindings.txt.

What we try to do here is to specify the hardware limitations. There
seem to be some hardware restrictions when it comes to clock
frequencies. There has been a rather long discussion over at Freescales
community about it:
https://community.freescale.com/thread/317074

Not sure if this is the right way to specify the supported frequencies,
or should we create a custom property for this, something like
fsl,max-nfc-frequency = 3300?


 
 BTW the above can easier be written as:
 
   of_property_read_u32(np, clock-frequency, cfg-clkrate);
 
 No return value checking necessary.
 
 +static int vf610_nfc_probe(struct platform_device *pdev)
 +{
 +struct vf610_nfc *nfc;
 +struct resource *res;
 +struct mtd_info *mtd;
 +struct nand_chip *chip;
 +struct vf610_nfc_config *cfg;
 +int err = 0;
 +int page_sz;
 +int irq;
 +
 +nfc = devm_kzalloc(pdev-dev, sizeof(*nfc), GFP_KERNEL);
 +if (!nfc)
 +return -ENOMEM;
 +
 +nfc-cfg = devm_kzalloc(pdev-dev, sizeof(*nfc), GFP_KERNEL);
 +if (!nfc-cfg)
 +return -ENOMEM;
 +cfg = nfc-cfg;
 
 Why is nfc-cfg allocated separately instead of embedding it into struct
 vf610_nfc? Is this some platform_data leftover you can remove now?
 

Yeah, looks like a platform_data leftover, will change that.

 +
 +nfc-dev = pdev-dev;
 +nfc-page = -1;
 +mtd = nfc-mtd;
 +chip = nfc-chip;
 +
 +mtd-priv = chip;
 +mtd-owner = THIS_MODULE;
 +mtd-dev.parent = nfc-dev;
 +mtd-name = DRV_NAME;
 +
 +err = vf610_nfc_probe_dt(nfc-dev, cfg);
 +if (err)
 +return -ENODEV;
 
 Does this driver work without device tree or not? Currently the driver
 bails out when device tree support is enabled but no device node is
 given. When device tree support is disabled in the kernel though the
 driver happily continues here.
 

Hm, I never tried using this Driver without DT. I guess in practice, we
won't get here in a DT enabled kernel anyway: If there is no device tree
node with one of the drivers compatibility ids, the probe function won't
be called.

Theoretically, the IP supported by this driver is part of two other
SoC's which are supported by Linux: PowerPC MPC5125 (DT) and a ColdFire
MCF54418 (non-DT). So this driver might be used on a ColdFire SoC using
the platform support. Currently, one would just not have the ability to
set the hardware ECC options... I'm a bit reluctant to implement full
platform support without being able to properly test it. If anybody is
interested in it, one could easily extend the driver...

Put Geert in CC...

--
Stefan

--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-06 Thread Bill Pringlemeir
On  6 Mar 2015, ste...@agner.ch wrote:

 On 2015-03-06 07:15, Sascha Hauer wrote:
 Hi Stefan,

 On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
 +
 +static int vf610_nfc_probe_dt(struct device *dev, struct
 vf610_nfc_config *cfg)
 +{
 +   struct device_node *np = dev-of_node;
 +   int buswidth;
 +   u32 clkrate;
 +
 +   if (!np)
 +   return 1;
 +
 +   cfg-flash_bbt = of_get_nand_on_flash_bbt(np);
 +
 +   if (!of_property_read_u32(np, clock-frequency, clkrate))
 +   cfg-clkrate = clkrate;

 Normally the clock-frequency property tells the driver at which
 frequency the device actually is running, not to tell the driver at
 which frequency the device *should* run. It's strange to use the
 value of the clock-frequency property as input to
 clk_set_rate(). Maybe the assigned clock binding is more appropriate
 here, see Documentation/devicetree/bindings/clock/clock-bindings.txt.

 What we try to do here is to specify the hardware limitations. There
 seem to be some hardware restrictions when it comes to clock
 frequencies. There has been a rather long discussion over at
 Freescales community about it:
 https://community.freescale.com/thread/317074

 Not sure if this is the right way to specify the supported
 frequencies, or should we create a custom property for this, something
 like fsl,max-nfc-frequency = 3300?

On most SOC's with this controller, the input clock to the controller
affects the NAND flash timing and other factors; so you will want to set
it based on the board/NAND stuffed.  The clock is for synchronous logic
in the controller and affects many properties.  

I guess Sascha's point is, the board's DT should just have some
'nfc_clk' node and not have this part of the driver?  Either way works.
However, this clock is very important to get the driver to function.  It
seem better for a user/porter to have this info in the node.  I guess
you need to be trained to look at every node in the sub-tree for a
device.  I think the other way might be better or a sub-system
maintainer.  I looked at 'i2c' and other node which have a
'clock-frequency' parameter. 

In the Documentation/devicetree/bindings/clock/clock-bindings.txt,

uart@a000 {
compatible = fsl,imx-uart;
reg = 0xa000 0x1000;
interrupts = 33;
clocks = osc 0, pll 1;
clock-names = baud, register;
};

Here, this uart clock may affect the maximum baud rate supported by the
device.  For this controller (vf610_nfc), the clock is like setting the
'baud rate'; it affect the NAND memory cycles.  There is not really any
'wait state' type logic in the controller register set that would allow
the driver to work with a 'given clock' rate.  For certain a board
should set this clock for the NAND chips they wish to support.

What would the board file look like to use clock node?

[generic]
nfc: nand@400e {
compatible = fsl,vf610-nfc;
reg = 0x400e 0x4000;
clocks = clks VF610_CLK_NFC;
clock-names = nfc_clk;
status = disabled;
};

[board]

nfc {
nand-bus-width = 16;
nand-ecc-mode = hw;
nand-on-flash-bbt;
nand-ecc-strength = 24;
nand-ecc-step-size = 2048;
pinctrl-names = default;
pinctrl-0 = pinctrl_nfc_1;
status = okay;
nfc_clk { frequency = 3300}  /* Like this? */
};

I don't know how to do the 'Like this?' part.  I don't think the
'clock-bindings.txt' explains it.  I see this is better as the the
driver needs no 'clock handling' code.  It is definitely a little more
obtuse for the users.

[snip]

 Does this driver work without device tree or not? Currently the
 driver bails out when device tree support is enabled but no device
 node is given. When device tree support is disabled in the kernel
 though the driver happily continues here.

 Hm, I never tried using this Driver without DT. 

[snip]

I also didn't test this.  The driver was ported from Linux versions
where DT did not exist.  It is used in some OpenWRT/68k/coldfire
(patched) kernels and I wanted it to be useful for them.  However, we
could probably remove the 'platform support'.  Other people are using
this on PPC platforms and they will also have dt/of.

Currently the platform control has no way to 'pass data', so the driver
only works with whatever defaults it has (or that is my belief).  For
instance, those OpenWRT kernels have a 'machine file' which will set the
'clock-frequency' and other parameters.  We could remove the platform
support completely if it is misleading.  I guess the KConfig would need
a 'depends on CONFIG_OF'.

Thanks for the review,
Bill Pringlemeir.
--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-05 Thread Sascha Hauer
Hi Stefan,

On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
> +
> +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config 
> *cfg)
> +{
> + struct device_node *np = dev->of_node;
> + int buswidth;
> + u32 clkrate;
> +
> + if (!np)
> + return 1;
> +
> + cfg->flash_bbt = of_get_nand_on_flash_bbt(np);
> +
> + if (!of_property_read_u32(np, "clock-frequency", ))
> + cfg->clkrate = clkrate;

Normally the clock-frequency property tells the driver at which
frequency the device actually is running, not to tell the driver at
which frequency the device *should* run. It's strange to use the value
of the clock-frequency property as input to clk_set_rate(). Maybe the
assigned clock binding is more appropriate here, see
Documentation/devicetree/bindings/clock/clock-bindings.txt.

BTW the above can easier be written as:

of_property_read_u32(np, "clock-frequency", >clkrate);

No return value checking necessary.

> +static int vf610_nfc_probe(struct platform_device *pdev)
> +{
> + struct vf610_nfc *nfc;
> + struct resource *res;
> + struct mtd_info *mtd;
> + struct nand_chip *chip;
> + struct vf610_nfc_config *cfg;
> + int err = 0;
> + int page_sz;
> + int irq;
> +
> + nfc = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc)
> + return -ENOMEM;
> +
> + nfc->cfg = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc->cfg)
> + return -ENOMEM;
> + cfg = nfc->cfg;

Why is nfc->cfg allocated separately instead of embedding it into struct
vf610_nfc? Is this some platform_data leftover you can remove now?

> +
> + nfc->dev = >dev;
> + nfc->page = -1;
> + mtd = >mtd;
> + chip = >chip;
> +
> + mtd->priv = chip;
> + mtd->owner = THIS_MODULE;
> + mtd->dev.parent = nfc->dev;
> + mtd->name = DRV_NAME;
> +
> + err = vf610_nfc_probe_dt(nfc->dev, cfg);
> + if (err)
> + return -ENODEV;

Does this driver work without device tree or not? Currently the driver
bails out when device tree support is enabled but no device node is
given. When device tree support is disabled in the kernel though the
driver happily continues here.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-05 Thread Shawn Guo
On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
> This driver supports Freescale NFC (NAND flash controller) found on
> Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.
> 
> Limitations:
> - DMA and pipelining not used
> - Pages larger than 2k are not supported
> - No hardware ECC
> 
> The driver has only been tested on Vybrid (VF610).
> 
> Signed-off-by: Bill Pringlemeir 
> Signed-off-by: Stefan Agner 
> ---
>  arch/arm/mach-imx/Kconfig|   1 +

This change shouldn't be part of driver patch.

Shawn

>  drivers/mtd/nand/Kconfig |  12 +
>  drivers/mtd/nand/Makefile|   1 +
>  drivers/mtd/nand/vf610_nfc.c | 730 
> +++
>  4 files changed, 744 insertions(+)
>  create mode 100644 drivers/mtd/nand/vf610_nfc.c
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index e8627e0..de4a51a 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -634,6 +634,7 @@ config SOC_VF610
>   select ARM_GIC
>   select PINCTRL_VF610
>   select PL310_ERRATA_769419 if CACHE_L2X0
> + select HAVE_NAND_VF610_NFC
>  
>   help
> This enable support for Freescale Vybrid VF610 processor.
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index 5b76a17..1be30a6 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
> This enables the driver for the NAND flash controller on the
> MPC5121 SoC.
>  
> +config HAVE_NAND_VF610_NFC
> + bool
> +
> +config MTD_NAND_VF610_NFC
> + tristate "Support for Freescale NFC for VF610/MPC5125"
> + depends on HAVE_NAND_VF610_NFC
> + help
> +   Enables support for NAND Flash Controller on some Freescale
> +   processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
> +   The driver supports a maximum 2k page size. The driver
> +   currently does not support hardware ECC.
> +
>  config MTD_NAND_MXC
>   tristate "MXC NAND support"
>   depends on ARCH_MXC
> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index 582bbd05..e97ca7b 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES) += 
> socrates_nand.o
>  obj-$(CONFIG_MTD_NAND_TXX9NDFMC) += txx9ndfmc.o
>  obj-$(CONFIG_MTD_NAND_NUC900)+= nuc900_nand.o
>  obj-$(CONFIG_MTD_NAND_MPC5121_NFC)   += mpc5121_nfc.o
> +obj-$(CONFIG_MTD_NAND_VF610_NFC) += vf610_nfc.o
>  obj-$(CONFIG_MTD_NAND_RICOH) += r852.o
>  obj-$(CONFIG_MTD_NAND_JZ4740)+= jz4740_nand.o
>  obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 000..101fd20
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,730 @@
> +/*
> + * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
> + *
> + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
> + * Jason ported to M54418TWR and MVFA5 (VF610).
> + * Authors: Stefan Agner 
> + *  Bill Pringlemeir 
> + *  Shaohui Xie 
> + *  Jason Jin 
> + *
> + * Based on original driver mpc5121_nfc.c.
> + *
> + * This is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * Limitations:
> + * - Untested on MPC5125 and M54418.
> + * - DMA not used.
> + * - 2K pages or less.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define  DRV_NAME"vf610_nfc"
> +
> +/* Register Offsets */
> +#define NFC_FLASH_CMD1   0x3F00
> +#define NFC_FLASH_CMD2   0x3F04
> +#define NFC_COL_ADDR 0x3F08
> +#define NFC_ROW_ADDR 0x3F0c
> +#define NFC_ROW_ADDR_INC 0x3F14
> +#define NFC_FLASH_STATUS10x3F18
> +#define NFC_FLASH_STATUS20x3F1c
> +#define NFC_CACHE_SWAP   0x3F28
> +#define NFC_SECTOR_SIZE  0x3F2c
> +#define NFC_FLASH_CONFIG 0x3F30
> +#define NFC_IRQ_STATUS   0x3F38
> +
> +/* Addresses for NFC MAIN RAM BUFFER areas */
> +#define NFC_MAIN_AREA(n) ((n) *  0x1000)
> +
> +#define PAGE_2K  0x0800
> +#define OOB_64   0x0040
> +
> +/*
> + * NFC_CMD2[CODE] values. See section:
> + *  - 31.4.7 Flash Command Code Description, Vybrid manual
> + *  - 23.8.6 Flash Command Sequencer, MPC5125 manual
> + *
> + * Briefly these are bitmasks of controller cycles.
> + */
> +#define READ_PAGE_CMD_CODE   0x7EE0
> +#define 

Re: [PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-05 Thread Paul Bolle
On Thu, 2015-03-05 at 00:10 +0100, Stefan Agner wrote:
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,730 @@

> + * This is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.

This states the license is GPL v2 or later.

> +MODULE_LICENSE("GPL v2");

So you probably want
MODULE_LICENSE("GPL");

here.


Paul Bolle

--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-05 Thread Paul Bolle
On Thu, 2015-03-05 at 00:10 +0100, Stefan Agner wrote:
 --- /dev/null
 +++ b/drivers/mtd/nand/vf610_nfc.c
 @@ -0,0 +1,730 @@

 + * This is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.

This states the license is GPL v2 or later.

 +MODULE_LICENSE(GPL v2);

So you probably want
MODULE_LICENSE(GPL);

here.


Paul Bolle

--
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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-05 Thread Sascha Hauer
Hi Stefan,

On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
 +
 +static int vf610_nfc_probe_dt(struct device *dev, struct vf610_nfc_config 
 *cfg)
 +{
 + struct device_node *np = dev-of_node;
 + int buswidth;
 + u32 clkrate;
 +
 + if (!np)
 + return 1;
 +
 + cfg-flash_bbt = of_get_nand_on_flash_bbt(np);
 +
 + if (!of_property_read_u32(np, clock-frequency, clkrate))
 + cfg-clkrate = clkrate;

Normally the clock-frequency property tells the driver at which
frequency the device actually is running, not to tell the driver at
which frequency the device *should* run. It's strange to use the value
of the clock-frequency property as input to clk_set_rate(). Maybe the
assigned clock binding is more appropriate here, see
Documentation/devicetree/bindings/clock/clock-bindings.txt.

BTW the above can easier be written as:

of_property_read_u32(np, clock-frequency, cfg-clkrate);

No return value checking necessary.

 +static int vf610_nfc_probe(struct platform_device *pdev)
 +{
 + struct vf610_nfc *nfc;
 + struct resource *res;
 + struct mtd_info *mtd;
 + struct nand_chip *chip;
 + struct vf610_nfc_config *cfg;
 + int err = 0;
 + int page_sz;
 + int irq;
 +
 + nfc = devm_kzalloc(pdev-dev, sizeof(*nfc), GFP_KERNEL);
 + if (!nfc)
 + return -ENOMEM;
 +
 + nfc-cfg = devm_kzalloc(pdev-dev, sizeof(*nfc), GFP_KERNEL);
 + if (!nfc-cfg)
 + return -ENOMEM;
 + cfg = nfc-cfg;

Why is nfc-cfg allocated separately instead of embedding it into struct
vf610_nfc? Is this some platform_data leftover you can remove now?

 +
 + nfc-dev = pdev-dev;
 + nfc-page = -1;
 + mtd = nfc-mtd;
 + chip = nfc-chip;
 +
 + mtd-priv = chip;
 + mtd-owner = THIS_MODULE;
 + mtd-dev.parent = nfc-dev;
 + mtd-name = DRV_NAME;
 +
 + err = vf610_nfc_probe_dt(nfc-dev, cfg);
 + if (err)
 + return -ENODEV;

Does this driver work without device tree or not? Currently the driver
bails out when device tree support is enabled but no device node is
given. When device tree support is disabled in the kernel though the
driver happily continues here.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-05 Thread Shawn Guo
On Thu, Mar 05, 2015 at 12:10:20AM +0100, Stefan Agner wrote:
 This driver supports Freescale NFC (NAND flash controller) found on
 Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.
 
 Limitations:
 - DMA and pipelining not used
 - Pages larger than 2k are not supported
 - No hardware ECC
 
 The driver has only been tested on Vybrid (VF610).
 
 Signed-off-by: Bill Pringlemeir bpringlem...@nbsps.com
 Signed-off-by: Stefan Agner ste...@agner.ch
 ---
  arch/arm/mach-imx/Kconfig|   1 +

This change shouldn't be part of driver patch.

Shawn

  drivers/mtd/nand/Kconfig |  12 +
  drivers/mtd/nand/Makefile|   1 +
  drivers/mtd/nand/vf610_nfc.c | 730 
 +++
  4 files changed, 744 insertions(+)
  create mode 100644 drivers/mtd/nand/vf610_nfc.c
 
 diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
 index e8627e0..de4a51a 100644
 --- a/arch/arm/mach-imx/Kconfig
 +++ b/arch/arm/mach-imx/Kconfig
 @@ -634,6 +634,7 @@ config SOC_VF610
   select ARM_GIC
   select PINCTRL_VF610
   select PL310_ERRATA_769419 if CACHE_L2X0
 + select HAVE_NAND_VF610_NFC
  
   help
 This enable support for Freescale Vybrid VF610 processor.
 diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
 index 5b76a17..1be30a6 100644
 --- a/drivers/mtd/nand/Kconfig
 +++ b/drivers/mtd/nand/Kconfig
 @@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
 This enables the driver for the NAND flash controller on the
 MPC5121 SoC.
  
 +config HAVE_NAND_VF610_NFC
 + bool
 +
 +config MTD_NAND_VF610_NFC
 + tristate Support for Freescale NFC for VF610/MPC5125
 + depends on HAVE_NAND_VF610_NFC
 + help
 +   Enables support for NAND Flash Controller on some Freescale
 +   processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
 +   The driver supports a maximum 2k page size. The driver
 +   currently does not support hardware ECC.
 +
  config MTD_NAND_MXC
   tristate MXC NAND support
   depends on ARCH_MXC
 diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
 index 582bbd05..e97ca7b 100644
 --- a/drivers/mtd/nand/Makefile
 +++ b/drivers/mtd/nand/Makefile
 @@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES) += 
 socrates_nand.o
  obj-$(CONFIG_MTD_NAND_TXX9NDFMC) += txx9ndfmc.o
  obj-$(CONFIG_MTD_NAND_NUC900)+= nuc900_nand.o
  obj-$(CONFIG_MTD_NAND_MPC5121_NFC)   += mpc5121_nfc.o
 +obj-$(CONFIG_MTD_NAND_VF610_NFC) += vf610_nfc.o
  obj-$(CONFIG_MTD_NAND_RICOH) += r852.o
  obj-$(CONFIG_MTD_NAND_JZ4740)+= jz4740_nand.o
  obj-$(CONFIG_MTD_NAND_GPMI_NAND) += gpmi-nand/
 diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
 new file mode 100644
 index 000..101fd20
 --- /dev/null
 +++ b/drivers/mtd/nand/vf610_nfc.c
 @@ -0,0 +1,730 @@
 +/*
 + * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
 + *
 + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
 + * Jason ported to M54418TWR and MVFA5 (VF610).
 + * Authors: Stefan Agner stefan.ag...@toradex.com
 + *  Bill Pringlemeir bpringlem...@nbsps.com
 + *  Shaohui Xie b21...@freescale.com
 + *  Jason Jin jason@freescale.com
 + *
 + * Based on original driver mpc5121_nfc.c.
 + *
 + * This is free software; you can redistribute it and/or modify it
 + * under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * Limitations:
 + * - Untested on MPC5125 and M54418.
 + * - DMA not used.
 + * - 2K pages or less.
 + */
 +
 +#include linux/module.h
 +#include linux/clk.h
 +#include linux/delay.h
 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/io.h
 +#include linux/mtd/mtd.h
 +#include linux/mtd/nand.h
 +#include linux/mtd/partitions.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/of_mtd.h
 +
 +#define  DRV_NAMEvf610_nfc
 +
 +/* Register Offsets */
 +#define NFC_FLASH_CMD1   0x3F00
 +#define NFC_FLASH_CMD2   0x3F04
 +#define NFC_COL_ADDR 0x3F08
 +#define NFC_ROW_ADDR 0x3F0c
 +#define NFC_ROW_ADDR_INC 0x3F14
 +#define NFC_FLASH_STATUS10x3F18
 +#define NFC_FLASH_STATUS20x3F1c
 +#define NFC_CACHE_SWAP   0x3F28
 +#define NFC_SECTOR_SIZE  0x3F2c
 +#define NFC_FLASH_CONFIG 0x3F30
 +#define NFC_IRQ_STATUS   0x3F38
 +
 +/* Addresses for NFC MAIN RAM BUFFER areas */
 +#define NFC_MAIN_AREA(n) ((n) *  0x1000)
 +
 +#define PAGE_2K  0x0800
 +#define OOB_64   0x0040
 +
 +/*
 + * NFC_CMD2[CODE] values. See section:
 + *  - 31.4.7 Flash Command Code Description, Vybrid manual
 + *  - 23.8.6 Flash 

[PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-04 Thread Stefan Agner
This driver supports Freescale NFC (NAND flash controller) found on
Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.

Limitations:
- DMA and pipelining not used
- Pages larger than 2k are not supported
- No hardware ECC

The driver has only been tested on Vybrid (VF610).

Signed-off-by: Bill Pringlemeir 
Signed-off-by: Stefan Agner 
---
 arch/arm/mach-imx/Kconfig|   1 +
 drivers/mtd/nand/Kconfig |  12 +
 drivers/mtd/nand/Makefile|   1 +
 drivers/mtd/nand/vf610_nfc.c | 730 +++
 4 files changed, 744 insertions(+)
 create mode 100644 drivers/mtd/nand/vf610_nfc.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e8627e0..de4a51a 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -634,6 +634,7 @@ config SOC_VF610
select ARM_GIC
select PINCTRL_VF610
select PL310_ERRATA_769419 if CACHE_L2X0
+   select HAVE_NAND_VF610_NFC
 
help
  This enable support for Freescale Vybrid VF610 processor.
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b76a17..1be30a6 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
  This enables the driver for the NAND flash controller on the
  MPC5121 SoC.
 
+config HAVE_NAND_VF610_NFC
+   bool
+
+config MTD_NAND_VF610_NFC
+   tristate "Support for Freescale NFC for VF610/MPC5125"
+   depends on HAVE_NAND_VF610_NFC
+   help
+ Enables support for NAND Flash Controller on some Freescale
+ processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+ The driver supports a maximum 2k page size. The driver
+ currently does not support hardware ECC.
+
 config MTD_NAND_MXC
tristate "MXC NAND support"
depends on ARCH_MXC
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 582bbd05..e97ca7b 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES)   += 
socrates_nand.o
 obj-$(CONFIG_MTD_NAND_TXX9NDFMC)   += txx9ndfmc.o
 obj-$(CONFIG_MTD_NAND_NUC900)  += nuc900_nand.o
 obj-$(CONFIG_MTD_NAND_MPC5121_NFC) += mpc5121_nfc.o
+obj-$(CONFIG_MTD_NAND_VF610_NFC)   += vf610_nfc.o
 obj-$(CONFIG_MTD_NAND_RICOH)   += r852.o
 obj-$(CONFIG_MTD_NAND_JZ4740)  += jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_GPMI_NAND)   += gpmi-nand/
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
new file mode 100644
index 000..101fd20
--- /dev/null
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -0,0 +1,730 @@
+/*
+ * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
+ *
+ * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
+ * Jason ported to M54418TWR and MVFA5 (VF610).
+ * Authors: Stefan Agner 
+ *  Bill Pringlemeir 
+ *  Shaohui Xie 
+ *  Jason Jin 
+ *
+ * Based on original driver mpc5121_nfc.c.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Limitations:
+ * - Untested on MPC5125 and M54418.
+ * - DMA not used.
+ * - 2K pages or less.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#defineDRV_NAME"vf610_nfc"
+
+/* Register Offsets */
+#define NFC_FLASH_CMD1 0x3F00
+#define NFC_FLASH_CMD2 0x3F04
+#define NFC_COL_ADDR   0x3F08
+#define NFC_ROW_ADDR   0x3F0c
+#define NFC_ROW_ADDR_INC   0x3F14
+#define NFC_FLASH_STATUS1  0x3F18
+#define NFC_FLASH_STATUS2  0x3F1c
+#define NFC_CACHE_SWAP 0x3F28
+#define NFC_SECTOR_SIZE0x3F2c
+#define NFC_FLASH_CONFIG   0x3F30
+#define NFC_IRQ_STATUS 0x3F38
+
+/* Addresses for NFC MAIN RAM BUFFER areas */
+#define NFC_MAIN_AREA(n)   ((n) *  0x1000)
+
+#define PAGE_2K0x0800
+#define OOB_64 0x0040
+
+/*
+ * NFC_CMD2[CODE] values. See section:
+ *  - 31.4.7 Flash Command Code Description, Vybrid manual
+ *  - 23.8.6 Flash Command Sequencer, MPC5125 manual
+ *
+ * Briefly these are bitmasks of controller cycles.
+ */
+#define READ_PAGE_CMD_CODE 0x7EE0
+#define PROGRAM_PAGE_CMD_CODE  0x7FC0
+#define ERASE_CMD_CODE 0x4EC0
+#define READ_ID_CMD_CODE   0x4804
+#define RESET_CMD_CODE 0x4040
+#define STATUS_READ_CMD_CODE   0x4068
+
+/* NFC ECC mode define */
+#define ECC_BYPASS 0
+
+/*** Register Mask and bit definitions */
+
+/* NFC_FLASH_CMD1 Field */

[PATCH 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

2015-03-04 Thread Stefan Agner
This driver supports Freescale NFC (NAND flash controller) found on
Vybrid (VF610), MPC5125, MCF54418 and Kinetis K70.

Limitations:
- DMA and pipelining not used
- Pages larger than 2k are not supported
- No hardware ECC

The driver has only been tested on Vybrid (VF610).

Signed-off-by: Bill Pringlemeir bpringlem...@nbsps.com
Signed-off-by: Stefan Agner ste...@agner.ch
---
 arch/arm/mach-imx/Kconfig|   1 +
 drivers/mtd/nand/Kconfig |  12 +
 drivers/mtd/nand/Makefile|   1 +
 drivers/mtd/nand/vf610_nfc.c | 730 +++
 4 files changed, 744 insertions(+)
 create mode 100644 drivers/mtd/nand/vf610_nfc.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e8627e0..de4a51a 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -634,6 +634,7 @@ config SOC_VF610
select ARM_GIC
select PINCTRL_VF610
select PL310_ERRATA_769419 if CACHE_L2X0
+   select HAVE_NAND_VF610_NFC
 
help
  This enable support for Freescale Vybrid VF610 processor.
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 5b76a17..1be30a6 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -455,6 +455,18 @@ config MTD_NAND_MPC5121_NFC
  This enables the driver for the NAND flash controller on the
  MPC5121 SoC.
 
+config HAVE_NAND_VF610_NFC
+   bool
+
+config MTD_NAND_VF610_NFC
+   tristate Support for Freescale NFC for VF610/MPC5125
+   depends on HAVE_NAND_VF610_NFC
+   help
+ Enables support for NAND Flash Controller on some Freescale
+ processors like the VF610, MPC5125, MCF54418 or Kinetis K70.
+ The driver supports a maximum 2k page size. The driver
+ currently does not support hardware ECC.
+
 config MTD_NAND_MXC
tristate MXC NAND support
depends on ARCH_MXC
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 582bbd05..e97ca7b 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_MTD_NAND_SOCRATES)   += 
socrates_nand.o
 obj-$(CONFIG_MTD_NAND_TXX9NDFMC)   += txx9ndfmc.o
 obj-$(CONFIG_MTD_NAND_NUC900)  += nuc900_nand.o
 obj-$(CONFIG_MTD_NAND_MPC5121_NFC) += mpc5121_nfc.o
+obj-$(CONFIG_MTD_NAND_VF610_NFC)   += vf610_nfc.o
 obj-$(CONFIG_MTD_NAND_RICOH)   += r852.o
 obj-$(CONFIG_MTD_NAND_JZ4740)  += jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_GPMI_NAND)   += gpmi-nand/
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
new file mode 100644
index 000..101fd20
--- /dev/null
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -0,0 +1,730 @@
+/*
+ * Copyright 2009-2015 Freescale Semiconductor, Inc. and others
+ *
+ * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver.
+ * Jason ported to M54418TWR and MVFA5 (VF610).
+ * Authors: Stefan Agner stefan.ag...@toradex.com
+ *  Bill Pringlemeir bpringlem...@nbsps.com
+ *  Shaohui Xie b21...@freescale.com
+ *  Jason Jin jason@freescale.com
+ *
+ * Based on original driver mpc5121_nfc.c.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Limitations:
+ * - Untested on MPC5125 and M54418.
+ * - DMA not used.
+ * - 2K pages or less.
+ */
+
+#include linux/module.h
+#include linux/clk.h
+#include linux/delay.h
+#include linux/init.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/mtd/mtd.h
+#include linux/mtd/nand.h
+#include linux/mtd/partitions.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include linux/of_mtd.h
+
+#defineDRV_NAMEvf610_nfc
+
+/* Register Offsets */
+#define NFC_FLASH_CMD1 0x3F00
+#define NFC_FLASH_CMD2 0x3F04
+#define NFC_COL_ADDR   0x3F08
+#define NFC_ROW_ADDR   0x3F0c
+#define NFC_ROW_ADDR_INC   0x3F14
+#define NFC_FLASH_STATUS1  0x3F18
+#define NFC_FLASH_STATUS2  0x3F1c
+#define NFC_CACHE_SWAP 0x3F28
+#define NFC_SECTOR_SIZE0x3F2c
+#define NFC_FLASH_CONFIG   0x3F30
+#define NFC_IRQ_STATUS 0x3F38
+
+/* Addresses for NFC MAIN RAM BUFFER areas */
+#define NFC_MAIN_AREA(n)   ((n) *  0x1000)
+
+#define PAGE_2K0x0800
+#define OOB_64 0x0040
+
+/*
+ * NFC_CMD2[CODE] values. See section:
+ *  - 31.4.7 Flash Command Code Description, Vybrid manual
+ *  - 23.8.6 Flash Command Sequencer, MPC5125 manual
+ *
+ * Briefly these are bitmasks of controller cycles.
+ */
+#define READ_PAGE_CMD_CODE 0x7EE0
+#define PROGRAM_PAGE_CMD_CODE  0x7FC0
+#define ERASE_CMD_CODE