| Message ID | 20171129005540.28829-3-david.daney@cavium.com |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Cavium OCTEON-III network driver. | expand |
On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote: > From: Carlos Munoz <cmunoz@cavium.com> > > LMTDMA/LMTST operations move data between cores and I/O devices: > > * LMTST operations can send an address and a variable length > (up to 128 bytes) of data to an I/O device. > * LMTDMA operations can send an address and a variable length > (up to 128) of data to the I/O device and then return a > variable length (up to 128 bytes) response from the IOI device. Should that be "I/O"? > > Signed-off-by: Carlos Munoz <cmunoz@cavium.com> > Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com> > Signed-off-by: David Daney <david.daney@cavium.com> > --- > arch/mips/cavium-octeon/setup.c | 6 ++++++ > arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++-- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c > index a8034d0dcade..99e6a68bc652 100644 > --- a/arch/mips/cavium-octeon/setup.c > +++ b/arch/mips/cavium-octeon/setup.c > @@ -609,6 +609,12 @@ void octeon_user_io_init(void) > #else > cvmmemctl.s.cvmsegenak = 0; > #endif > + if (OCTEON_IS_OCTEON3()) { > + /* Enable LMTDMA */ > + cvmmemctl.s.lmtena = 1; > + /* Scratch line to use for LMT operation */ > + cvmmemctl.s.lmtline = 2; Out of curiosity, is there significance to the value 2 and associated virtual address 0xffffffffffff8100, or is it pretty arbitrary? > + } > /* R/W If set, CVMSEG is available for loads/stores in > * supervisor mode. */ > cvmmemctl.s.cvmsegenas = 0; > diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h > index c99c4b6a79f4..92a17d67c1fa 100644 > --- a/arch/mips/include/asm/octeon/octeon.h > +++ b/arch/mips/include/asm/octeon/octeon.h > @@ -179,7 +179,15 @@ union octeon_cvmemctl { > /* RO 1 = BIST fail, 0 = BIST pass */ > __BITFIELD_FIELD(uint64_t wbfbist:1, > /* Reserved */ > - __BITFIELD_FIELD(uint64_t reserved:17, > + __BITFIELD_FIELD(uint64_t reserved_52_57:6, > + /* When set, LMTDMA/LMTST operations are permitted */ > + __BITFIELD_FIELD(uint64_t lmtena:1, > + /* Selects the CVMSEG LM cacheline used by LMTDMA > + * LMTST and wide atomic store operations. > + */ > + __BITFIELD_FIELD(uint64_t lmtline:6, > + /* Reserved */ > + __BITFIELD_FIELD(uint64_t reserved_41_44:4, > /* OCTEON II - TLB replacement policy: 0 = bitmask LRU; 1 = NLU. > * This field selects between the TLB replacement policies: > * bitmask LRU or NLU. Bitmask LRU maintains a mask of > @@ -275,7 +283,7 @@ union octeon_cvmemctl { > /* R/W Size of local memory in cache blocks, 54 (6912 > * bytes) is max legal value. */ > __BITFIELD_FIELD(uint64_t lmemsz:6, > - ;))))))))))))))))))))))))))))))))) > + ;)))))))))))))))))))))))))))))))))))) > } s; > }; Regardless, the patch looks good to me. Reviewed-by: James Hogan <jhogan@kernel.org> Cheers James
On 11/30/2017 01:36 PM, James Hogan wrote: > On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote: >> From: Carlos Munoz <cmunoz@cavium.com> >> >> LMTDMA/LMTST operations move data between cores and I/O devices: >> >> * LMTST operations can send an address and a variable length >> (up to 128 bytes) of data to an I/O device. >> * LMTDMA operations can send an address and a variable length >> (up to 128) of data to the I/O device and then return a >> variable length (up to 128 bytes) response from the IOI device. > > Should that be "I/O"? Yes, I will fix the changelog. > >> >> Signed-off-by: Carlos Munoz <cmunoz@cavium.com> >> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com> >> Signed-off-by: David Daney <david.daney@cavium.com> >> --- >> arch/mips/cavium-octeon/setup.c | 6 ++++++ >> arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++-- >> 2 files changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c >> index a8034d0dcade..99e6a68bc652 100644 >> --- a/arch/mips/cavium-octeon/setup.c >> +++ b/arch/mips/cavium-octeon/setup.c >> @@ -609,6 +609,12 @@ void octeon_user_io_init(void) >> #else >> cvmmemctl.s.cvmsegenak = 0; >> #endif >> + if (OCTEON_IS_OCTEON3()) { >> + /* Enable LMTDMA */ >> + cvmmemctl.s.lmtena = 1; >> + /* Scratch line to use for LMT operation */ >> + cvmmemctl.s.lmtline = 2; > > Out of curiosity, is there significance to the value 2 and associated > virtual address 0xffffffffffff8100, or is it pretty arbitrary? Yes, there is significance. CPU local memory starts at 0xffffffffffff8000, each line is 0x80 bytes. so the 2nd line starts at 0xffffffffffff8100 > >> + } >> /* R/W If set, CVMSEG is available for loads/stores in >> * supervisor mode. */ >> cvmmemctl.s.cvmsegenas = 0; >> diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h >> index c99c4b6a79f4..92a17d67c1fa 100644 >> --- a/arch/mips/include/asm/octeon/octeon.h >> +++ b/arch/mips/include/asm/octeon/octeon.h >> @@ -179,7 +179,15 @@ union octeon_cvmemctl { >> /* RO 1 = BIST fail, 0 = BIST pass */ >> __BITFIELD_FIELD(uint64_t wbfbist:1, >> /* Reserved */ >> - __BITFIELD_FIELD(uint64_t reserved:17, >> + __BITFIELD_FIELD(uint64_t reserved_52_57:6, >> + /* When set, LMTDMA/LMTST operations are permitted */ >> + __BITFIELD_FIELD(uint64_t lmtena:1, >> + /* Selects the CVMSEG LM cacheline used by LMTDMA >> + * LMTST and wide atomic store operations. >> + */ >> + __BITFIELD_FIELD(uint64_t lmtline:6, >> + /* Reserved */ >> + __BITFIELD_FIELD(uint64_t reserved_41_44:4, >> /* OCTEON II - TLB replacement policy: 0 = bitmask LRU; 1 = NLU. >> * This field selects between the TLB replacement policies: >> * bitmask LRU or NLU. Bitmask LRU maintains a mask of >> @@ -275,7 +283,7 @@ union octeon_cvmemctl { >> /* R/W Size of local memory in cache blocks, 54 (6912 >> * bytes) is max legal value. */ >> __BITFIELD_FIELD(uint64_t lmemsz:6, >> - ;))))))))))))))))))))))))))))))))) >> + ;)))))))))))))))))))))))))))))))))))) >> } s; >> }; > > Regardless, the patch looks good to me. > > Reviewed-by: James Hogan <jhogan@kernel.org> > > Cheers > James >
On Thu, Nov 30, 2017 at 01:49:43PM -0800, David Daney wrote: > On 11/30/2017 01:36 PM, James Hogan wrote: > > On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote: > >> Signed-off-by: Carlos Munoz <cmunoz@cavium.com> > >> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com> > >> Signed-off-by: David Daney <david.daney@cavium.com> > >> --- > >> arch/mips/cavium-octeon/setup.c | 6 ++++++ > >> arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++-- > >> 2 files changed, 16 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c > >> index a8034d0dcade..99e6a68bc652 100644 > >> --- a/arch/mips/cavium-octeon/setup.c > >> +++ b/arch/mips/cavium-octeon/setup.c > >> @@ -609,6 +609,12 @@ void octeon_user_io_init(void) > >> #else > >> cvmmemctl.s.cvmsegenak = 0; > >> #endif > >> + if (OCTEON_IS_OCTEON3()) { > >> + /* Enable LMTDMA */ > >> + cvmmemctl.s.lmtena = 1; > >> + /* Scratch line to use for LMT operation */ > >> + cvmmemctl.s.lmtline = 2; > > > > Out of curiosity, is there significance to the value 2 and associated > > virtual address 0xffffffffffff8100, or is it pretty arbitrary? > > Yes, there is significance. > > CPU local memory starts at 0xffffffffffff8000, each line is 0x80 bytes. > so the 2nd line starts at 0xffffffffffff8100 What I mean is, why is 2 chosen instead of any other value? Cheers James
On 11/30/2017 02:56 PM, James Hogan wrote: > On Thu, Nov 30, 2017 at 01:49:43PM -0800, David Daney wrote: >> On 11/30/2017 01:36 PM, James Hogan wrote: >>> On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote: >>>> Signed-off-by: Carlos Munoz <cmunoz@cavium.com> >>>> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com> >>>> Signed-off-by: David Daney <david.daney@cavium.com> >>>> --- >>>> arch/mips/cavium-octeon/setup.c | 6 ++++++ >>>> arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++-- >>>> 2 files changed, 16 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c >>>> index a8034d0dcade..99e6a68bc652 100644 >>>> --- a/arch/mips/cavium-octeon/setup.c >>>> +++ b/arch/mips/cavium-octeon/setup.c >>>> @@ -609,6 +609,12 @@ void octeon_user_io_init(void) >>>> #else >>>> cvmmemctl.s.cvmsegenak = 0; >>>> #endif >>>> + if (OCTEON_IS_OCTEON3()) { >>>> + /* Enable LMTDMA */ >>>> + cvmmemctl.s.lmtena = 1; >>>> + /* Scratch line to use for LMT operation */ >>>> + cvmmemctl.s.lmtline = 2; >>> >>> Out of curiosity, is there significance to the value 2 and associated >>> virtual address 0xffffffffffff8100, or is it pretty arbitrary? >> >> Yes, there is significance. >> >> CPU local memory starts at 0xffffffffffff8000, each line is 0x80 bytes. >> so the 2nd line starts at 0xffffffffffff8100 > > What I mean is, why is 2 chosen instead of any other value? That is explained in the change log of patch 5/8: 1st 128-bytes: Use by IOBDMA 2nd 128-bytes: Reserved by kernel for scratch/TLS emulation. 3rd 128-bytes: OCTEON-III LMTLINE > > Cheers > James >
On Thu, Nov 30, 2017 at 03:09:33PM -0800, David Daney wrote: > On 11/30/2017 02:56 PM, James Hogan wrote: > > On Thu, Nov 30, 2017 at 01:49:43PM -0800, David Daney wrote: > >> On 11/30/2017 01:36 PM, James Hogan wrote: > >>> On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote: > >>>> Signed-off-by: Carlos Munoz <cmunoz@cavium.com> > >>>> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com> > >>>> Signed-off-by: David Daney <david.daney@cavium.com> > >>>> --- > >>>> arch/mips/cavium-octeon/setup.c | 6 ++++++ > >>>> arch/mips/include/asm/octeon/octeon.h | 12 ++++++++++-- > >>>> 2 files changed, 16 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c > >>>> index a8034d0dcade..99e6a68bc652 100644 > >>>> --- a/arch/mips/cavium-octeon/setup.c > >>>> +++ b/arch/mips/cavium-octeon/setup.c > >>>> @@ -609,6 +609,12 @@ void octeon_user_io_init(void) > >>>> #else > >>>> cvmmemctl.s.cvmsegenak = 0; > >>>> #endif > >>>> + if (OCTEON_IS_OCTEON3()) { > >>>> + /* Enable LMTDMA */ > >>>> + cvmmemctl.s.lmtena = 1; > >>>> + /* Scratch line to use for LMT operation */ > >>>> + cvmmemctl.s.lmtline = 2; > >>> > >>> Out of curiosity, is there significance to the value 2 and associated > >>> virtual address 0xffffffffffff8100, or is it pretty arbitrary? > >> > >> Yes, there is significance. > >> > >> CPU local memory starts at 0xffffffffffff8000, each line is 0x80 bytes. > >> so the 2nd line starts at 0xffffffffffff8100 > > > > What I mean is, why is 2 chosen instead of any other value? > > That is explained in the change log of patch 5/8: > > > 1st 128-bytes: Use by IOBDMA > 2nd 128-bytes: Reserved by kernel for scratch/TLS emulation. > 3rd 128-bytes: OCTEON-III LMTLINE Ah yes. Perhaps it deserves a brief comment in the code, or even an enum. Cheers James
diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c index a8034d0dcade..99e6a68bc652 100644 --- a/arch/mips/cavium-octeon/setup.c +++ b/arch/mips/cavium-octeon/setup.c @@ -609,6 +609,12 @@ void octeon_user_io_init(void) #else cvmmemctl.s.cvmsegenak = 0; #endif + if (OCTEON_IS_OCTEON3()) { + /* Enable LMTDMA */ + cvmmemctl.s.lmtena = 1; + /* Scratch line to use for LMT operation */ + cvmmemctl.s.lmtline = 2; + } /* R/W If set, CVMSEG is available for loads/stores in * supervisor mode. */ cvmmemctl.s.cvmsegenas = 0; diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h index c99c4b6a79f4..92a17d67c1fa 100644 --- a/arch/mips/include/asm/octeon/octeon.h +++ b/arch/mips/include/asm/octeon/octeon.h @@ -179,7 +179,15 @@ union octeon_cvmemctl { /* RO 1 = BIST fail, 0 = BIST pass */ __BITFIELD_FIELD(uint64_t wbfbist:1, /* Reserved */ - __BITFIELD_FIELD(uint64_t reserved:17, + __BITFIELD_FIELD(uint64_t reserved_52_57:6, + /* When set, LMTDMA/LMTST operations are permitted */ + __BITFIELD_FIELD(uint64_t lmtena:1, + /* Selects the CVMSEG LM cacheline used by LMTDMA + * LMTST and wide atomic store operations. + */ + __BITFIELD_FIELD(uint64_t lmtline:6, + /* Reserved */ + __BITFIELD_FIELD(uint64_t reserved_41_44:4, /* OCTEON II - TLB replacement policy: 0 = bitmask LRU; 1 = NLU. * This field selects between the TLB replacement policies: * bitmask LRU or NLU. Bitmask LRU maintains a mask of @@ -275,7 +283,7 @@ union octeon_cvmemctl { /* R/W Size of local memory in cache blocks, 54 (6912 * bytes) is max legal value. */ __BITFIELD_FIELD(uint64_t lmemsz:6, - ;))))))))))))))))))))))))))))))))) + ;)))))))))))))))))))))))))))))))))))) } s; };