Re: [PATCH v3 5/7] dt-bindings: scsi: ufs: Add document for ufs-mediatek

[Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>]

Hi Rob,

On Thu, 2019-03-14 at 03:48 +0800, Rob Herring wrote:
> On Tue, Mar 12, 2019 at 9:41 PM Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >
> > Hi Rob,
> >
> > On Tue, 2019-03-12 at 21:31 +0800, Rob Herring wrote:
> > > On Wed, Feb 27, 2019 at 03:23:10PM +0800, Stanley Chu wrote:
> > > > Add UFS and UFS PHY node document for Mediatek SoC chips.
> > > >
> > > > Signed-off-by: Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > ---
> > > >  .../devicetree/bindings/ufs/ufs-mediatek.txt  | 47 +++++++++++++++++++
> > > >  1 file changed, 47 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/ufs/ufs-mediatek.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/ufs/ufs-mediatek.txt b/Documentation/devicetree/bindings/ufs/ufs-mediatek.txt
> > > > new file mode 100644
> > > > index 000000000000..5fc985928b3e
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/ufs/ufs-mediatek.txt
> > > > @@ -0,0 +1,47 @@
> > > > +* Mediatek Universal Flash Storage (UFS) Host Controller
> > > > +
> > > > +UFS nodes are defined to describe on-chip UFS hardware macro.
> > > > +Each UFS Host Controller should have its own node.
> > > > +
> > > > +To bind UFS PHY with UFS host controller, the controller node should
> > > > +contain a phandle reference to UFS M-PHY node.
> > > > +
> > > > +Required properties for UFS nodes:
> > > > +- compatible         : Compatible list, contains the following controller:
> > > > +                       "mediatek,ufshci"
> > >
> > > Needs an SoC specific compatible string.
> >
> > Will add.
> >
> > >
> > > > +- reg                : Address and length of the UFS register set.
> > > > +- interrupt-parent   : Interrupt device.
> > >
> > > Drop this. It is implied or may be in a parent node.
> >
> > Will remove it.
> >
> > >
> > > > +- phys               : phandle to m-phy.
> > > > +- clocks             : List of phandle and clock specifier pairs.
> > > > +- clock-names        : List of clock input name strings sorted in the same
> > > > +                       order as the clocks property. "ufs-clk" is mandatory.
> > > > +- freq-table-hz      : Array of <min max> operating frequencies stored in the same
> > > > +                       order as the clocks property. If this property is not
> > > > +                       defined or a value in the array is "0" then it is assumed
> > > > +                       that the frequency is set by the parent clock or a
> > > > +                       fixed rate clock source.
> > > > +- vcc-supply         : Power to the UFS device.
> > > > +- vcc-fixed-regulator: Specify that vcc-supply is a fixed regulator.
> > >
> > > Why is this needed? The driver could query the voltage range of the
> > > regulator or you could check the regulator node.
> >
> > Both ufshcd-pltfrm.txt and ufshcd-pltfrm.c driver allow
> >
> > - <name>-fixed-regulator : boolean property specifying that
> > <name>-supply is a fixed regulator.
> >
> > Our vcc does not require further initializatio, so is it OK here or
> > shall we also modify both ufshcd-pltfrm dt-bindings document and ufs
> > driver to avoid using it ?
> 
> There are no users of that property in tree and doesn't look like
> adding it was ever reviewed. We have standard ways to handle this and
> don't need a custom property.

Got it and thanks.

I am planning to drop VCC related first in this series and add it back
by another patch accompanied with VCC/ICC logic fix in UFS common driver
which beyond the scope of this series.

> 
> Rob

Thanks,
Stanley.