Re: [Xen-devel] [XEN/ARM PATCH v6 1/1] Add OdroidXU board (Exynos 5410)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-09-10 at 10:05 -0700, Suriyan Ramasami wrote:
> On Wed, Sep 10, 2014 at 7:07 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Thu, 2014-09-04 at 15:57 -0700, Suriyan Ramasami wrote:
> >
> >>      BUILD_BUG_ON(EXYNOS5_MCT_G_TCON >= PAGE_SIZE);
> >>
> >> -    mct = ioremap_attr(EXYNOS5_MCT_BASE, PAGE_SIZE, 
> >> PAGE_HYPERVISOR_NOCACHE);
> >> +    node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-mct");
> >> +    if ( !node )
> >> +    {
> >> +        dprintk(XENLOG_ERR, "samsung,exynos4210-mct missing in DT\n");
> >> +        return -ENXIO;
> >> +    }
> >> +
> >> +    rc = dt_device_get_address(node, 0, &mct_base_addr, &size);
> >> +    if ( rc )
> >> +    {
> >> +        dprintk(XENLOG_ERR, "Error in \"samsung,exynos4210-mct\"\n");
> >> +        return -ENXIO;
> >> +    }
> >> +
> >> +    dprintk(XENLOG_INFO, "mct_base_addr: %016llx size: %016llx\n",
> >> +            mct_base_addr, size);
> >> +
> >> +    mct = ioremap_attr(mct_base_addr, PAGE_SIZE, PAGE_HYPERVISOR_NOCACHE);
> >
> > I noticed this while applying, it wasn't enough to stop me but it would
> > be nice to fix:
> >
> > We should drop that BUILD_BUG_ON and use the size retrieved from the DT
> > for the mapping instead.
> >
> This did occur to me, more so when mapping the power base address (as
> I map from an offset rather than base so that I am within the
> PAGE_SIZE that I map). From what you are suggesting, we should just
> map the base address (mct or pmu) for the size that we retrieve. I see
> that the mct is of size 0x800 and the pmu is of size 0x5000.
> We can then add a check to see if the offsets that we are cooking up,
> like EXYNOS5_MCT_G_TCON are within size that we have retrieved.
> I hope this is what you were suggesting. Please let me know, and I
> shall send a patch for this.

Yes, I think the logic should be:
      * Retrieve address+size from DT
      * Check that size is large enough for the registers we need to
        touch, error if not
      * Map size bytes from address
      * Access the registers using appropriate offsets

IOW do the check before bothering to map, and map the returned address,
don't add the offset on map, add it on access instead.

Thanks,
Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel