Re: [PATCH 3/3] add support for DWC UFS Host Controller

From: Mark Rutland <mark.rutland@arm.com>
To: Joao Pinto <Joao.Pinto@synopsys.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	santosh.sy@samsung.com, h.vinayak@samsung.com,
	julian.calaby@gmail.com, akinobu.mita@gmail.com,
	hch@infradead.org, gbroner@codeaurora.org,
	subhashj@codeaurora.org, CARLOS.PALMINHA@synopsys.com,
	ijc+devicetree@hellion.org.uk, linux-kernel@vger.kernel.org,
	linux-scsi@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 3/3] add support for DWC UFS Host Controller
Date: Thu, 4 Feb 2016 16:27:01 +0000	[thread overview]
Message-ID: <20160204162700.GD17587@leverpostej> (raw)
In-Reply-To: <56B222C8.5010509@synopsys.com>

On Wed, Feb 03, 2016 at 03:54:48PM +0000, Joao Pinto wrote:
> Hi,
> 
> On 2/3/2016 3:39 PM, Arnd Bergmann wrote:
> > On Wednesday 03 February 2016 15:01:34 Joao Pinto wrote:
> >>
> >> Hi Arnd,
> >>
> >> On 2/3/2016 12:54 PM, Arnd Bergmann wrote:
> >>> On Wednesday 03 February 2016 11:28:26 Joao Pinto wrote:
> >>>>
> >>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> >>>
> >>> This needs a changelog comment, like every patch.
> >>>
> >>>> @@ -0,0 +1,16 @@
> >>>> +* Universal Flash Storage (UFS) DesignWare Host Controller
> >>>> +
> >>>> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
> >>>> +Each UFS controller instance should have its own node.
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible        : compatible list, contains "snps,ufshcd"
> >>>
> >>> Are there multiple versions of this controller? Usually for designware
> >>> parts the version is known, so we should document which versions exist
> >>
> >> This controller recent releases was 2.0, but we released last year 1.1. The
> >> driver works with both. The driver must work with all DWC UFS versions.
> > 
> > Ok, then make the driver match on the "snps,ufshcd-1.1" compatible
> > string, but document both strings in the binding document, and make
> > it mandatory to specify the 1.1 version as a compatible fallback.
> > 
> > If we ever need to handle a quirk for the 2.0 version then, it can
> > easily be done.
> 
> We need the driver to support UFS 2.0 because it is our latest release and is
> the done that Synopsys is focused now. We could call it "snps, ufshcd-2.0" then.
> What do you think?

Arnd's point was that the driver can handle only "snps,ufshcd-1.1" for
now, and in your DT you can have:

	compatible = "snps,ufshcd-2.0", "snps,ufshcd-1.1";

That allows driver to handle 2.0 and 1.1 without knowing anything about
2.0 for now. If in future the two need to be handled differently we can
update the driver to explicitly match "snps,ufshcd-2.0".

Regardless, both compatible string should go in the documentation, and
it should be explicitly mentioned that "snps,ufshcd-1.1" should be used
as a fallback entry.

Mark.