Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 2 Jun 2016 21:03:02 +0200
From:      Zbigniew Bodek <zbb@semihalf.com>
To:        Ian Lepore <ian@freebsd.org>, =?UTF-8?Q?Micha=C5=82_Stanek?= <mst@semihalf.com>
Cc:        Zbigniew Bodek <zbb@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r301220 - in head/sys: arm/mv dev/cesa
Message-ID:  <CAG7dG%2ByO5sBJ_4W-XcvOtjx_iAJz79WruGptM6Pe5BciB2OrHw@mail.gmail.com>
In-Reply-To: <1464893290.1204.186.camel@freebsd.org>
References:  <201606021831.u52IVb1O006883@repo.freebsd.org> <1464893290.1204.186.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
2016-06-02 20:48 GMT+02:00 Ian Lepore <ian@freebsd.org>:

> On Thu, 2016-06-02 at 18:31 +0000, Zbigniew Bodek wrote:
> > Author: zbb
> > Date: Thu Jun  2 18:31:36 2016
> > New Revision: 301220
> > URL: https://svnweb.freebsd.org/changeset/base/301220
> >
> > Log:
> >   Map CESA SRAM memory in driver attach for Armada38x
> >
> >   On other platforms with CESA accelerator the SRAM memory is mapped
> > in
> >   early init before driver is attached. This method only works
> > correctly
> >   with mappings no smaller than L1 section size (1MB). There may be
> > more
> >   SRAM blocks and they may have smaller sizes than 1MB as is the case
> >   for Armada38x. Instead, map SRAM memory with bus_space_map() in
> > CESA
> >   driver attach. Note that we can no longer assume that VA == PA for
> > the
> >   SRAM.
> >
> >   Submitted by:       Michal Stanek <mst@semihalf.com
> >   Obtained from:      Semihalf
> >   Sponsored by:       Stormshield
> >   Differential revision:      https://reviews.freebsd.org/D6215
> > [...]
> > -
> > +     rv = OF_getprop(sram_node, "reg", (void *)sram_reg,
> > sizeof(sram_reg));
> > +     if (rv <= 0)
> > +             return (rv);
> > +
> > +     sc->sc_sram_base_pa = fdt32_to_cpu(sram_reg[0]);
> > +     /* Store SRAM size to be able to unmap in detach() */
> > +     sc->sc_sram_size = fdt32_to_cpu(sram_reg[1]);
> > +
>
> OF_getprop() followed by fdt32_to_cpu() calls is properly spelled
> OF_getencprop() (with no fdt32_to_cpu calls).
>
> > +#if defined(SOC_MV_ARMADA38X)
> > +     /* SRAM memory was not mapped in platform_sram_devmap(), map
> > it now */
> > +     rv = bus_space_map(fdtbus_bs_tag, sc->sc_sram_base_pa, sc
> > ->sc_sram_size,
> > +         0, &(sc->sc_sram_base_va));
>
> bus_space_map() returns a bus_space_handle_t for use with other
> bus_space functions.  The handle is not necessarily "just the virtual
> address" (although that happens to be the case right now on arm).  I
> don't see any bus_space_xxxxx() calls using this handle, that means
> that probably the correct function to use is pmap_mapdev(), not
> bus_space_map().
>
> -- Ian
>
> Thanks Ian,

We will fix this ASAP.
BTW. It would be better to get this review prior to committing the patch
;-)
Phabricator revision didn't attract anyone's attention:
https://reviews.freebsd.org/D6215

Kind regards
zbb



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG7dG%2ByO5sBJ_4W-XcvOtjx_iAJz79WruGptM6Pe5BciB2OrHw>