Re: [PATCH v2 1/4] spi: imx: GPIO based chip selects should not be required

[Trent Piepho <tpiepho-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2922 bytes --]

On Fri, 2017-11-03 at 19:36 +0000, Mark Brown wrote:
> On Fri, Nov 03, 2017 at 07:18:56PM +0000, Trent Piepho wrote:
> > On Fri, 2017-11-03 at 18:37 +0000, Mark Brown wrote:
> > > If there are non-standard bindings then mark them as deprecated.  I
> > > can't immediately find *any* binding documentation for this controller.
> > > The last commit looks like it was more attempting to work round broken
> > > board bindings and do something sensible than add a new binding, at
> > > least that's what I remember my sense of it being.
> > The non-standard part is needing to add cs-gpios = <0> to get a native
> > chip select when that is documented as being optional.  It doesn't
> > follow the spec.  It doesn't match other drivers (and dw-spi is equally
> > as broken in the same manner, probably others too) that do follow the
> > spec.
> 
> Is there any documentation of the bindings for this driver at all?  I
> wasn't able to find it.

Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt

Doesn't note anything special about the native chip selects.

> > > If the hardware is as broken as these controllers always were in the
> > > past and there are workarounds which work in all practical situations
> > > (AFAIK all the relevant SoCs permit GPIO usage on the chip select pins)
> > Comments in the driver indicate that some SoCs do not allow GPIO usage
> > on all chip select pins.
> 
> Ah, that's an issue.  We will need support for the hardware chip selects
> then.

And they are already supported too.  The issue I want to fix is the
need for non-standard bindings to use them.

> > > documenting something as supported is just going to make people
> > > miserable.  The reason I know about this breakage is that I had to go
> > > through the process of working out that the native chip select support
> > > didn't work on a system.
> > I just don't see how not following the device tree binding
> > specification documents the hardware flaw, or how following the spec
> > documents that the flaw does not exist.
> 
> Hardware chip selects aren't present in all controllers and at times
> have entertaining enumerations in the hardware, the details on them need
> to be covered in the device specific binding (which like I say seems to
> be missing for this controller).  If they just don't work well the
> kindest thing may be to not support them and document the binding that
> way.
> 
> > > Yes, better documentation would be great.
> > How about I add something to Documentation/spi and add a note in
> > Kconfig, but make the driver standard compliant in its device tree
> > bindings?
> 
> Writing a binding document for the controller would probably cover it.

Ok, I'll adjust the series to document the real issue.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±²˜¢žØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&¢îý»\x05ËÛÔØï¦v¬Îf\x1dp)¹¹br	šê+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹\x1e®w¥¢¸?™¨è­Ú&¢)ߢ^[f