On Fri, Oct 11, 2013 at 11:54:52AM +0100, Valentine wrote:
> On 10/11/2013 01:41 PM, Mark Rutland wrote:
> > On Fri, Oct 11, 2013 at 02:00:35AM +0100, Simon Horman wrote:
> >> [ CCed devicetree@xxxxxxxxxxxxxxx as this involves DT compatibility strings ]
> >
> > Cheers!
> >
>
> Hi Mark,
>
> >>
> >> On Thu, Oct 10, 2013 at 11:08:03PM +0400, Valentine Barshak wrote:
> >>> RCAR Gen2 SoC has a different phy which is not compatible with
> >>> the older H1/M1 versions. This adds OF/platform device table
> >>> and PHY initialization callbacks for H2/M2 (Gen2) SoC.
> >
> > Is the PHY combined with the rest of the controller, or are they
> > logically separate components in the SoC? I note that the Calxeda
> > Highbank SATA controller driver treats the phy and the controller as
> > separate entities, and describes the way the two are attached (though
> > the driver handles both). Would a similar approach work here?
> >
>
> It seems to be described in the docs as a single entity with the rest of
> the controller. The phy registers are in the same address space block.
> We don't need to describe how the phy is attached to the SATA
> controller. In the future we may need some phy-specific configuration in
> the device tree. For example, to describe how clock generator is
> connected to the controller. I think this could be done with optional
> properties added to the SATA node if needed.
As they're tightly coupled and share the same register block, I guess
describing them together is ok. Is that clock example hypothetical, or
something we _will_ need in future?
>
> > [...]
> >
> >>> +static struct of_device_id sata_rcar_match[] = {
> >>> + {
> >>> + .compatible = "renesas,rcar-sata",
> >>> + .data = (void *)RCAR_SATA
> >>> + },
> >>> + {
> >>> + .compatible = "renesas,sata-r8a7790",
> >>> + .data = (void *)RCAR_GEN2_SATA
> >>> + },
> >>> + {
> >>> + .compatible = "renesas,sata-r8a7791",
> >>> + .data = (void *)RCAR_GEN2_SATA
> >>> + },
> >>> + {},
> >
> > These bindings will need documentation. A grep of any of these in
> > mainline's Documentation/devicetree shows up nothing (not even the
> > existing "renesas,rcar-sata" string used by the driver.
>
> Indeed.
> Since we need to adjust rcar-sata bindings and add documentation for it,
> which is not really related to Gen2 phy support, looks like it will be a
> separate patch.
If you're adding strings, there should be documentation. While it may be
a separate patch, we should _not_ add compatible strings to the kernel
without documentation. I'd like to see the documentation patch first.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
