Date:   Mon, 13 Feb 2023 03:24:39 -0800
From:   Saurabh Singh Sengar <ssengar@linux.microsoft.com>
To:     Rob Herring <robh+dt@kernel.org>
Cc:     krzysztof.kozlowski+dt@linaro.org, kys@microsoft.com,
        haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
        daniel.lezcano@linaro.org, tglx@linutronix.de,
        virtualization@lists.linux-foundation.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-hyperv@vger.kernel.org, mikelley@microsoft.com,
        dphadke@linux.microsoft.com
Subject: Re: [PATCH v5 5/5] Driver: VMBus: Add device tree support
Message-ID: <20230213112439.GA15305@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1675944939-22631-1-git-send-email-ssengar@linux.microsoft.com>
 <1675944939-22631-6-git-send-email-ssengar@linux.microsoft.com>
 <CAL_JsqK0WgWm-mG=fYyDVAi4uhL+fM0OD7KEF+xYYOOGNX8-oQ@mail.gmail.com>
 <20230209174641.GB1346@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
 <CAL_JsqK+4ecEmGyo0jVs3B-E6Rjj4Lo8vB0En6pEUR1P4cRXpA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAL_JsqK+4ecEmGyo0jVs3B-E6Rjj4Lo8vB0En6pEUR1P4cRXpA@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Fri, Feb 10, 2023 at 04:37:28PM -0600, Rob Herring wrote:
> On Thu, Feb 9, 2023 at 11:46 AM Saurabh Singh Sengar
> <ssengar@linux.microsoft.com> wrote:
> >
> > On Thu, Feb 09, 2023 at 09:50:31AM -0600, Rob Herring wrote:
> > > On Thu, Feb 9, 2023 at 6:15 AM Saurabh Sengar
> > > <ssengar@linux.microsoft.com> wrote:
> > > >
> > > > Update the driver to support device tree boot as well along with ACPI.
> > >
> > > Devicetree
> 
> [...]
> 
> > > > +       for_each_of_range(&parser, &range) {
> > > > +               struct resource *res;
> > > > +
> > > > +               res = kzalloc(sizeof(*res), GFP_ATOMIC);
> > > > +               if (!res)
> > > > +                       return -ENOMEM;
> > > > +
> > > > +               res->name = "hyperv mmio";
> > > > +               res->flags = IORESOURCE_MEM | IORESOURCE_MEM_64;
> > > > +               res->start = range.pci_addr;
> > >
> > > This is not PCI. It's a union, so use 'bus_addr' instead.
> > >
> > > But wait, resources and IORESOURCE_MEM are *CPU* addresses. You need
> > > cpu_addr here. Your DT happens to do 1:1 addresses so it happens to
> > > work either way.
> >
> > bus_addr works for us, will send V6
> 
> Sigh. bus_addr may work, but is wrong as I explained.
> 
> And you've already sent v6... Please slow down your pace with sending
> new versions. 4 versions in a week is too much. Give others time to
> comment and me to respond to discussions. Like a week...

I apologize if my actions may have come across as overly hasty. I will make
sure to allow for more time between submissions in the future, to ensure that
everyone has an adequate opportunity to review and provide feedback.

Regarding the use of bus_addr instead of cpu_addr, I found that cpu_addr was
populating as OF_BAD_ADDR while bus_addr was populating correctly.  I think
this is because I should be defining a empty ranges property in parent node
for indicating 1:1 mapping between parent and child.

But once I add empty ranges in property I get other warnings by dt_binding_check
tool. After fixing all I am able to come up with below device tree example, please
let me know if there is anything to be corrected. If this is good I will send
the next version (offcource after a week :)) using cpu_addr.

    soc {
        #address-cells = <2>;
        #size-cells = <1>;
        bus {
            compatible = "simple-bus";
            #address-cells = <2>;
            #size-cells = <1>;
            ranges;

            vmbus@ff0000000 {
                compatible = "microsoft,vmbus";
                #address-cells = <2>;
                #size-cells = <1>;
                ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>;
            };
        };
    };

> 
> Rob