Re: [PATCH] [MTD] [NAND] GPIO NAND flash driver

Mike Frysinger
2008-10-12T08:56:39Z

On Sun, Oct 12, 2008 at 04:28, Russell King - ARM Linux wrote:
> On Sun, Oct 12, 2008 at 04:14:43AM -0400, Mike Frysinger wrote:
>> On Sun, Oct 12, 2008 at 04:02, Mike Rapoport wrote:
>> > + if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
>> > + gpio_set_value(gpiomtd->plat.gpio_nwp, 0);
>> > + gpio_set_value(gpiomtd->plat.gpio_nce, 1);
>> > +
>> > + gpio_free(gpiomtd->plat.gpio_cle);
>> > + gpio_free(gpiomtd->plat.gpio_ale);
>> > + gpio_free(gpiomtd->plat.gpio_nce);
>> > + if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
>> > + gpio_free(gpiomtd->plat.gpio_nwp);
>>
>> why do you bother setting the value before releasing ? when you
>> release, the pin tends to go to the hardware default and on the
>> Blackfin, that tends to be "tristate". are you just banking on the
>> idea that the pin will stay the way it was last set before it gets
>> freed ?
>
> Maybe you should reconsider that behaviour - this is a prime example
> why such behaviour is wrong. If you leave the NAND signals floating,
> you're going to eat power - most CMOS based devices want inputs to be
> either logic high or low, and not floating otherwise they eat power.
>
> Moreover, you don't want to leave memory control lines floating - you
> don't want them to pick up noise which may be transmitted inside the
> NAND chip and may cause malfunction.
>
> Lastly, you don't want spurious writes to the NAND memory region (think
> DMA controller scribbling over memory because of some buggy driver) to
> write into the NAND, or maybe cause the NAND to erase itself.
>
> There's another reason for doing this. Think GPIO line connected to
> a loudspeaker amplifier shutdown input. Do you really want that line
> floating when the sound driver isn't loaded?

this is what pull downs/pull ups are for. your arguments can just as
readily be applied to the powering up state and initialization stages.
software cant (and shouldnt) rectify crappy hardware designs.

> On ARM, certainly PXA, the last GPIO state is preserved when free'd.

if certain behavior is expected, then it should be codified. i see no
mention (unless i just missed it) of expected behavior upon freeing in
Documentation/gpio.txt.
-mike