Re: [PATCH V3 1/2] dt-bindings: PCI: tegra234: Add ECAM support

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]

On Tue, Dec 06, 2022 at 03:14:58PM -0600, Rob Herring wrote:
> On Tue, Dec 6, 2022 at 10:44 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Mon, Dec 05, 2022 at 05:41:55PM -0600, Rob Herring wrote:
> > > On Thu, Nov 17, 2022 at 3:38 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> > > >
> > > > On Mon, Nov 14, 2022 at 03:53:32PM +0000, Jon Hunter wrote:
> > > > > From: Vidya Sagar <vidyas@nvidia.com>
> > > > >
> > > > > Add support for ECAM aperture that is only supported for Tegra234
> > > > > devices.
> > > > >
> > > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> > > > > Co-developed-by: Jon Hunter <jonathanh@nvidia.com>
> > > > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> > > > > ---
> > > > > Changes since V2:
> > > > > - Avoid duplication of reg items and reg-names
> > > > > Changes since V1:
> > > > > - Restricted the ECAM aperture to only Tegra234 devices that support it.
> > > > >
> > > > >  .../bindings/pci/nvidia,tegra194-pcie.yaml    | 34 +++++++++++++++++--
> > > > >  .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
> > > > >  2 files changed, 33 insertions(+), 3 deletions(-)
> > > >
> > > > Both patches applied now.
> > >
> > > linux-next now fails with this. I suspect it is due to Sergey's
> > > changes to the DWC schema.
> > >
> > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
> > > pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
> > > fixed:
> > >         'dbi' was expected
> > >         'dbi2' was expected
> > >         'ecam' is not one of ['elbi', 'app']
> > >         'atu' was expected
> > >         'dma' was expected
> > >         'phy' was expected
> > >         'config' was expected
> > >         /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.example.dtb:
> > > pcie@14160000: reg-names:4: 'oneOf' conditional failed, one must be
> > > fixed:
> > >                 'ecam' is not one of ['apb', 'mgmt', 'link', 'ulreg', 'appl']
> > >                 'ecam' is not one of ['atu_dma']
> > >                 'ecam' is not one of ['smu', 'mpu']
> > >         From schema:
> > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> >
> > Stephen reported the other day that he wasn't able to resolve this
> > conflict in linux-next, so he dropped the ECAM bits. The ECAM patch has
> > now propagated to ARM SoC so it can't be easily backed out, but I guess
> > we could revert on that tree and instead apply the patch to the DT tree
> > and resolve the conflict there.
> >
> > I guess the better alternative would be to try and resolve the merge
> > properly and let Stephen (and Linus) know.
> 
> Instead, can you prepare a patch on top of Sergey's adding a 'oneOf'
> entry with 'ecam'. As this is a new thing, it should have its own
> entry. Then when merging, we just throw out the change from your side.
> 
> I'd really prefer that bindings don't go thru the soc tree unless
> there's some strong reason. The default is to go via the subsystem
> trees. Beyond 'we are running the dtschema checks on all our dts files
> and can't have the warnings', I don't know what that would be. I wish
> everyone was doing that, but I'm pretty sure most are not.

That's actually the reason why I wanted this to go through ARM SoC, so
that the validation tools wouldn't accumulate even more errors. For some
time now I've been working on getting at least Tegra to validate
properly and on 64-bit ARM I have a local tree on top of linux-next that
has no warnings left, though I do need to flush out quite a few bindings
conversions.

Until those are all upstream, that argument is a little moot and don't
feel strongly about where the DT bindings ultimately end up.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]