Re: [PATCH v2] staging: xgifb: remove macros with hidden variable

[Dan Carpenter]

On Thu, Nov 30, 2017 at 10:39:48AM -0500, Joshua Abraham wrote:
> diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/XGI_main_26.c
> index 6feecc55d2bc..6de66eaad96b 100644
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -34,16 +34,16 @@ static void dumpVGAReg(struct xgifb_video_info 
> *xgifb_info)
>  {
>   u8 i, reg;
>  
> - xgifb_reg_set(XGISR, 0x05, 0x86);
> + xgifb_reg_set(xgifb_info->dev_info.P3c4, 0x05, 0x86);

This patch is OK, but it might be nicer to create a temporary variable
so the lines are not so long:

struct vb_device_info *vb = _info->dev_info;
u8 i, reg;

xgifb_reg_set(vb->P3c4, 0x05, 0x86);

I chose "vb" based on the struct name...  "dev" and "info" aren't very
useful in a name because there are a lot of devices and lots of types
of info.

regards,
dan carpneter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/5] mtd: nand: force drivers to explicitly send READ/PROG commands

[Masahiro Yamada]

2017-12-01 2:01 GMT+09:00 Miquel Raynal :
> From: Boris Brezillon 
>
> The core currently send the READ0 and SEQIN+PAGEPROG commands in
> nand_do_read/write_ops(). This is inconsistent with
> ->read/write_oob[_raw]() hooks behavior which are expected to send
> these commands.
>
> There's already a flag (NAND_ECC_CUSTOM_PAGE_ACCESS) to inform the core
> that a specific controller wants to send the READ/SEQIN+PAGEPROG
> commands on its own, but it's an opt-in flag, and existing drivers are
> unlikely to be updated to pass it.
>
> Moreover, some controllers cannot dissociate the READ/PAGEPROG commands
> from the associated data transfer and ECC engine activation, and
> developers have to hack things in their ->cmdfunc() implementation to
> handle such complex cases, or have to accept the perf penalty of sending
> twice the same command.
> To address this problem we are planning on adding a new interface which
> is passed all information about a NAND operation (including the amount
> of data to transfer) and replacing all calls to ->cmdfunc() to calls to
> this new ->exec_op() hook. But, in order to do that, we need to have all
> ->cmdfunc() calls placed near their associated ->read/write_buf/byte()
> calls.
>
> Modify the core and relevant drivers to make NAND_ECC_CUSTOM_PAGE_ACCESS
> the default case, and remove this flag.
>
> Signed-off-by: Boris Brezillon 
> [miquel.ray...@free-electrons.com: tested, fixed and rebased on nand/next]
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/mtd/nand/atmel/nand-controller.c  |  7 ++-
>  drivers/mtd/nand/bf5xx_nand.c |  6 +-
>  drivers/mtd/nand/brcmnand/brcmnand.c  | 13 +++-
>  drivers/mtd/nand/cafe_nand.c  |  6 +-
>  drivers/mtd/nand/denali.c |  1 -

For denali.c

Acked-by: Masahiro Yamada 


-- 
Best Regards
Masahiro Yamada
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/5] mtd: nand: provide several helpers to do common NAND operations

[Masahiro Yamada]

2017-12-01 2:01 GMT+09:00 Miquel Raynal :
> From: Boris Brezillon 
>
> This is part of the process of removing direct calls to ->cmdfunc()
> outside of the core in order to introduce a better interface to execute
> NAND operations.
>
> Here we provide several helpers and make use of them to remove all
> direct calls to ->cmdfunc(). This way, we can easily modify those
> helpers to make use of the new ->exec_op() interface when available.
>
> Signed-off-by: Boris Brezillon 
> [miquel.ray...@free-electrons.com: rebased and fixed some conflicts]
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/mtd/nand/atmel/nand-controller.c |2 +-
>  drivers/mtd/nand/brcmnand/brcmnand.c |9 +-
>  drivers/mtd/nand/cafe_nand.c |   14 +-
>  drivers/mtd/nand/denali.c|   37 +-

For denali.c

Acked-by: Masahiro Yamada 


-- 
Best Regards
Masahiro Yamada
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/5] mtd: nand: use usual return values for the ->erase() hook

[Masahiro Yamada]

2017-12-01 7:02 GMT+09:00 Miquel RAYNAL :
>> > diff --git a/drivers/mtd/nand/nand_base.c
>> > b/drivers/mtd/nand/nand_base.c index 630048f5abdc..4d1f2bda6095
>> > 100644 --- a/drivers/mtd/nand/nand_base.c
>> > +++ b/drivers/mtd/nand/nand_base.c
>> > @@ -3077,7 +3077,7 @@ int nand_erase_nand(struct mtd_info *mtd,
>> > struct erase_info *instr, status = chip->erase(mtd, page &
>> > chip->pagemask);
>> > /* See if block erase succeeded */
>> > -   if (status & NAND_STATUS_FAIL) {
>> > +   if (status) {
>> > pr_debug("%s: failed erase, page 0x%08x\n",
>> > __func__, page);
>> > instr->state = MTD_ERASE_FAILED;
>>
>> You forgot to patch single_erase() accordingly.
>
> Right, sorry about that, I will fix that.
>

Assuming single_erase() will be fixed,


For denali.c

Acked-by: Masahiro Yamada 





-- 
Best Regards
Masahiro Yamada
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

[David Daney]

On 11/30/2017 02:53 PM, James Hogan wrote:

On Tue, Nov 28, 2017 at 04:55:35PM -0800, David Daney wrote:

From: Carlos Munoz 

Add a global resource manager to manage tagged pointers within
bootmem allocated memory. This is used by various functional
blocks in the Octeon core like the FPA, Ethernet nexus, etc.

Signed-off-by: Carlos Munoz 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
  arch/mips/cavium-octeon/Makefile   |   3 +-
  arch/mips/cavium-octeon/resource-mgr.c | 371 +
  arch/mips/include/asm/octeon/octeon.h  |  18 ++
  3 files changed, 391 insertions(+), 1 deletion(-)
  create mode 100644 arch/mips/cavium-octeon/resource-mgr.c

diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
index 7c02e542959a..0a299ab8719f 100644
--- a/arch/mips/cavium-octeon/Makefile
+++ b/arch/mips/cavium-octeon/Makefile
@@ -9,7 +9,8 @@
  # Copyright (C) 2005-2009 Cavium Networks
  #
  
-obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o

+obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o \
+resource-mgr.o


Maybe put that on a separate line like below.


OK




  obj-y += dma-octeon.o
  obj-y += octeon-memcpy.o
  obj-y += executive/
diff --git a/arch/mips/cavium-octeon/resource-mgr.c 
b/arch/mips/cavium-octeon/resource-mgr.c
new file mode 100644
index ..ca25fa953402
--- /dev/null
+++ b/arch/mips/cavium-octeon/resource-mgr.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Resource manager for Octeon.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2017 Cavium, Inc.
+ */
+#include 
+
+#include 
+#include 
+
+#define RESOURCE_MGR_BLOCK_NAME"cvmx-global-resources"
+#define MAX_RESOURCES  128
+#define INST_AVAILABLE -88
+#define OWNER  0xbadc0de
+
+struct global_resource_entry {
+   struct global_resource_tag tag;
+   u64 phys_addr;
+   u64 size;
+};
+
+struct global_resources {
+#ifdef __LITTLE_ENDIAN_BITFIELD
+   u32 rlock;
+   u32 pad;
+#else
+   u32 pad;
+   u32 rlock;
+#endif
+   u64 entry_cnt;
+   struct global_resource_entry resource_entry[];
+};
+
+static struct global_resources *res_mgr_info;
+
+
+/*
+ * The resource manager interacts with software running outside of the
+ * Linux kernel, which necessitates locking to maintain data structure
+ * consistency.  These custom locking functions implement the locking
+ * protocol, and cannot be replaced by kernel locking functions that
+ * may use different in-memory structures.
+ */
+
+static void res_mgr_lock(void)
+{
+   unsigned int tmp;
+   u64 lock = (u64)_mgr_info->rlock;


presumably this could be a u32 *, avoid the cast to u64, and still work
just fine below.


I will rewrite to just use cmpxchg()





+
+   __asm__ __volatile__(
+   ".set noreorder\n"
+   "1: ll   %[tmp], 0(%[addr])\n"
+   "   bnez %[tmp], 1b\n"
+   "   li   %[tmp], 1\n"


I believe the convention for .S files is for instructions in branch
delay slots to be indented an additional space for readability. Maybe
that would be worthwhile here.


+   "   sc   %[tmp], 0(%[addr])\n"
+   "   beqz %[tmp], 1b\n"
+   "   nop\n"


and here also.


+   ".set reorder\n" :


nit: strictly speaking there's no need for \n on the last line.


+   [tmp] "="(tmp) :
+   [addr] "r"(lock) :
+   "memory");


minor style thing: its far more common to have : at the beginning of the
line rather than the end.


+}
+
+static void res_mgr_unlock(void)
+{
+   u64 lock = (u64)_mgr_info->rlock;


same again



Will rewrite to use WRITE_ONCE().


+
+   /* Wait until all resource operations finish before unlocking. */
+   mb();
+   __asm__ __volatile__(
+   "sw $0, 0(%[addr])\n" : :
+   [addr] "r"(lock) :
+   "memory");
+
+   /* Force a write buffer flush. */
+   mb();
+}
+
+static int res_mgr_find_resource(struct global_resource_tag tag)
+{
+   struct global_resource_entry *res_entry;
+   int i;
+
+   for (i = 0; i < res_mgr_info->entry_cnt; i++) {
+   res_entry = _mgr_info->resource_entry[i];
+   if (res_entry->tag.lo == tag.lo && res_entry->tag.hi == tag.hi)
+   return i;
+   }
+   return -1;
+}
+
+/**
+ * res_mgr_create_resource - Create a resource.
+ * @tag: Identifies the resource.
+ * @inst_cnt: Number of resource instances to create.
+ *
+ * Returns 0 if the source was created successfully.
+ * Returns <0 for error codes.


Only -1 seems to be returned. Is it worth 

Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.

[James Hogan]

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 
>  Signed-off-by: Steven J. Hill 
>  Signed-off-by: David Daney 
>  ---
> 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 0x8100, or is it pretty arbitrary?
> >>
> >> Yes, there is significance.
> >>
> >> CPU local memory starts at 0x8000, each line is 0x80 bytes.
> >> so the 2nd line starts at 0x8100
> > 
> > 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


signature.asc
Description: Digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.

[David Daney]

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 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
   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 0x8100, or is it pretty arbitrary?


Yes, there is significance.

CPU local memory starts at 0x8000, each line is 0x80 bytes.
so the 2nd line starts at 0x8100


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



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.

[James Hogan]

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 
> >> Signed-off-by: Steven J. Hill 
> >> Signed-off-by: David Daney 
> >> ---
> >>   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 0x8100, or is it pretty arbitrary?
> 
> Yes, there is significance.
> 
> CPU local memory starts at 0x8000, each line is 0x80 bytes. 
> so the 2nd line starts at 0x8100

What I mean is, why is 2 chosen instead of any other value?

Cheers
James


signature.asc
Description: Digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/5] mtd: nand: add ->exec_op() implementation

[Miquel RAYNAL]

> > diff --git a/drivers/mtd/nand/nand_base.c
> > b/drivers/mtd/nand/nand_base.c index 52965a8aeb2c..46bf31aff909
> > 100644 --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -689,6 +689,59 @@ static void nand_wait_status_ready(struct
> > mtd_info *mtd, unsigned long timeo) };
> >  
> >  /**
> > + * nand_soft_waitrdy - Read the status waiting for it to be ready
> > + * @chip: NAND chip structure
> > + * @timeout_ms: Timeout in ms
> > + *
> > + * Poll the status using ->exec_op() until it is ready unless it
> > takes too
> > + * much time.
> > + *
> > + * This helper is intended to be used by drivers without R/B pin
> > available to
> > + * poll for the chip status until ready and may be called at any
> > time in the
> > + * middle of any set of instruction. The READ_STATUS just need to
> > ask a single
> > + * time for it and then any read will return the status. Once the
> > READ_STATUS
> > + * cycles are done, the function will send a READ0 command to
> > cancel the
> > + * "READ_STATUS state" and let the normal flow of operation to
> > continue.
> > + *
> > + * This helper *cannot* send a WAITRDY command or ->exec_op()
> > implementations  
> 
> ^ instruction
> 
> > + * using it will enter an infinite loop.  
> 
> Hm, not sure why this would be the case, but okay. Maybe you should
> move this comment outside the kernel doc header, since this is an
> implementation detail, not something the caller/user should be aware
> of.

Right.

> 
> There's another important aspect to mention here: this function can
> only be called from an ->exec_op() implementation if this
> implementation is re-entrant.

I do not agree with this statement: this function can be called from an
->exec_op() implementation even if it is not reentrant as long as it
does not send a WAITRDY instruction itself. No?

Or maybe you wanted to point that the entire ->exec_op()
implementation must be reentrant in order to use this function in it?

> 
> > + *
> > + * Return 0 if the NAND chip is ready, a negative error otherwise.
> > + */
> > +int nand_soft_waitrdy(struct nand_chip *chip, unsigned long
> > timeout_ms) +{
> > +   u8 status = 0;
> > +   int ret;
> > +
> > +   if (!chip->exec_op)
> > +   return -ENOTSUPP;
> > +
> > +   ret = nand_status_op(chip, NULL);
> > +   if (ret)
> > +   return ret;
> > +
> > +   timeout_ms = jiffies + msecs_to_jiffies(timeout_ms);
> > +   do {
> > +   ret = nand_read_data_op(chip, ,
> > sizeof(status), true);
> > +   if (ret)
> > +   break;
> > +
> > +   if (status & NAND_STATUS_READY)
> > +   break;
> > +
> > +   udelay(100);  
> 
> Sounds a bit high, especially for a read page which takes around 20us.

Well, this value is arbitrary but greping for NAND_OP_WAIT_RDY tells us
the different timeouts with which this function is usually called, to
get an idea of the possible wait periods: tR, tBERS, tFEAT, tPROG, tRST.

While a tR_max is 200us, a tRST_max is 25us. That is why I choose
100us as period, which I found somehow well tuned for every timeout. But
if you think most of the time the delay will be smaller, I will update
the value to repeat the operation every 20us.

> 
> > +   } while (time_before(jiffies, timeout_ms));
> > +
> > +   nand_exit_status_op(chip);
> > +
> > +   if (ret)
> > +   return ret;
> > +
> > +   return status & NAND_STATUS_READY ? 0 : -ETIMEDOUT;
> > +};
> > +EXPORT_SYMBOL_GPL(nand_soft_waitrdy);
> > +
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/5] mtd: nand: use usual return values for the ->erase() hook

[Miquel RAYNAL]

> > diff --git a/drivers/mtd/nand/nand_base.c
> > b/drivers/mtd/nand/nand_base.c index 630048f5abdc..4d1f2bda6095
> > 100644 --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -3077,7 +3077,7 @@ int nand_erase_nand(struct mtd_info *mtd,
> > struct erase_info *instr, status = chip->erase(mtd, page &
> > chip->pagemask); 
> > /* See if block erase succeeded */
> > -   if (status & NAND_STATUS_FAIL) {
> > +   if (status) {
> > pr_debug("%s: failed erase, page 0x%08x\n",
> > __func__, page);
> > instr->state = MTD_ERASE_FAILED;  
> 
> You forgot to patch single_erase() accordingly.

Right, sorry about that, I will fix that.

Thanks,
Miquèl
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.

[David Daney]

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 

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 
Signed-off-by: Steven J. Hill 
Signed-off-by: David Daney 
---
  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 0x8100, or is it pretty arbitrary?


Yes, there is significance.

CPU local memory starts at 0x8000, each line is 0x80 bytes. 
so the 2nd line starts at 0x8100






+   }
/* 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 

Cheers
James



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] media: atomisp: convert default struct values to use compound-literals with designated initializers.

[Jeremy Sowden]

The CSS API uses a lot of nested anonymous structs defined in object
macros to assign default values to its data-structures.  These have been
changed to use compound-literals and designated initializers to make
them more comprehensible and less fragile.

The compound-literals can also be used in assignment, which means we can
get rid of some temporary variables whose only purpose is to be
initialized by one of these anonymous structs and then serve as the
rvalue in an assignment expression.

Signed-off-by: Jeremy Sowden 
---
 .../hive_isp_css_common/input_formatter_global.h   |  25 ++--
 .../pci/atomisp2/css2400/ia_css_frame_public.h |  46 ---
 .../atomisp/pci/atomisp2/css2400/ia_css_pipe.h | 145 +++--
 .../pci/atomisp2/css2400/ia_css_pipe_public.h  | 136 +--
 .../atomisp/pci/atomisp2/css2400/ia_css_types.h|  78 +--
 .../isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h | 130 +-
 .../kernels/sdis/common/ia_css_sdis_common_types.h |  85 +---
 .../isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c   |   3 +-
 .../runtime/binary/interface/ia_css_binary.h   | 140 ++--
 .../atomisp2/css2400/runtime/binary/src/binary.c   |   3 +-
 .../isp_param/interface/ia_css_isp_param_types.h   |  19 ++-
 .../runtime/pipeline/interface/ia_css_pipeline.h   |  30 ++---
 .../css2400/runtime/pipeline/src/pipeline.c|   7 +-
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c|  22 +---
 .../atomisp/pci/atomisp2/css2400/sh_css_legacy.h   |  16 +--
 .../atomisp/pci/atomisp2/css2400/sh_css_metrics.h  |  26 ++--
 16 files changed, 514 insertions(+), 397 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
index 5654d911db65..d5a586b08955 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
@@ -108,19 +108,20 @@ struct input_formatter_cfg_s {
 };
 
 #define DEFAULT_IF_CONFIG \
+(struct input_formatter_cfg_s) \
 { \
-   0,  /* start_line */\
-   0,  /* start_column */\
-   0,  /* left_padding */\
-   0,  /* cropped_height */\
-   0,  /* cropped_width */\
-   0,  /* deinterleaving */\
-   0,  /*.buf_vecs */\
-   0,  /* buf_start_index */\
-   0,  /* buf_increment */\
-   0,  /* buf_eol_offset */\
-   false,  /* is_yuv420_format */\
-   false   /* block_no_reqs */\
+   .start_line = 0, \
+   .start_column   = 0, \
+   .left_padding   = 0, \
+   .cropped_height = 0, \
+   .cropped_width  = 0, \
+   .deinterleaving = 0, \
+   .buf_vecs   = 0, \
+   .buf_start_index= 0, \
+   .buf_increment  = 0, \
+   .buf_eol_offset = 0, \
+   .is_yuv420_format   = false, \
+   .block_no_reqs  = false \
 }
 
 extern const hrt_address HIVE_IF_SRST_ADDRESS[N_INPUT_FORMATTER_ID];
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h
index 92f2389176b2..786585037af9 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h
@@ -121,15 +121,19 @@ struct ia_css_frame_info {
 };
 
 #define IA_CSS_BINARY_DEFAULT_FRAME_INFO \
-{ \
-   {0,  /* width */ \
-0}, /* height */ \
-   0,   /* padded_width */ \
-   IA_CSS_FRAME_FORMAT_NUM, /* format */ \
-   0,   /* raw_bit_depth */ \
-   IA_CSS_BAYER_ORDER_NUM,  /* raw_bayer_order */ \
-   {0,   /*start col */ \
-0},   /*start line */ \
+(struct ia_css_frame_info) { \
+   .res= (struct ia_css_resolution) { \
+   .width = 0, \
+   .height = 0 \
+   }, \
+   .padded_width   = 0, \
+   .format = IA_CSS_FRAME_FORMAT_NUM,  \
+   .raw_bit_depth  = 0, \
+   .raw_bayer_order= IA_CSS_BAYER_ORDER_NUM, \
+   .crop_info  = (struct ia_css_crop_info) { \
+   .start_column   = 0, \
+   .start_line = 0 \
+   }, \
 }
 
 /**
@@ -190,18 +194,18 @@ struct ia_css_frame {
 };
 
 #define DEFAULT_FRAME \
-{ \
-   IA_CSS_BINARY_DEFAULT_FRAME_INFO,   /* 

[PATCH 3/3] media: atomisp: delete empty default struct values.

[Jeremy Sowden]

Removing zero-valued struct-members left a number of the default
struct-values empty.  These values have now been removed.

Signed-off-by: Jeremy Sowden 
---
 .../atomisp/pci/atomisp2/css2400/ia_css_pipe.h |  1 -
 .../atomisp/pci/atomisp2/css2400/ia_css_types.h|  1 -
 .../isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h |  3 --
 .../kernels/sdis/common/ia_css_sdis_common_types.h | 34 +-
 .../isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c   |  2 +-
 .../runtime/binary/interface/ia_css_binary.h   |  6 
 .../isp_param/interface/ia_css_isp_param_types.h   |  9 --
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c|  9 +++---
 .../atomisp/pci/atomisp2/css2400/sh_css_legacy.h   |  2 --
 .../atomisp/pci/atomisp2/css2400/sh_css_metrics.h  |  8 -
 10 files changed, 12 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h
index a8f89866876d..9c6efaa66c93 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h
@@ -164,7 +164,6 @@ struct ia_css_pipe {
 #define IA_CSS_DEFAULT_PIPE \
 (struct ia_css_pipe) { \
.config = DEFAULT_PIPE_CONFIG, \
-   .extra_config   = DEFAULT_PIPE_EXTRA_CONFIG, \
.info   = DEFAULT_PIPE_INFO, \
.mode   = IA_CSS_PIPE_ID_ACC, /* (pipe_id) */ \
.pipeline   = DEFAULT_PIPELINE, \
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_types.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_types.h
index 0ae60a3d0643..e49fe5bc98b8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_types.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_types.h
@@ -407,7 +407,6 @@ struct ia_css_grid_info {
 /** defaults for ia_css_grid_info structs */
 #define DEFAULT_GRID_INFO \
 (struct ia_css_grid_info) { \
-   .s3a_grid   = DEFAULT_3A_GRID_INFO, \
.dvs_grid   = DEFAULT_DVS_GRID_INFO, \
.vamem_type = IA_CSS_VAMEM_TYPE_1 \
 }
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h
index 1309b06747d0..4297c3addcb2 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h
@@ -98,9 +98,6 @@ struct ia_css_3a_grid_info {
 };
 
 
-#define DEFAULT_3A_GRID_INFO (struct ia_css_3a_grid_info) { }
-
-
 /* This struct should be split into 3, for AE, AWB and AF.
  * However, that will require driver/ 3A lib modifications.
  */
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/common/ia_css_sdis_common_types.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/common/ia_css_sdis_common_types.h
index a969384430aa..4cfd31bd3e5f 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/common/ia_css_sdis_common_types.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/isp/kernels/sdis/common/ia_css_sdis_common_types.h
@@ -41,8 +41,6 @@ struct ia_css_sdis_info {
uint32_t deci_factor_log2;
 };
 
-#define IA_CSS_DEFAULT_SDIS_INFO (struct ia_css_sdis_info) { }
-
 /** DVS statistics grid
  *
  *  ISP block: SDVS1 (DIS/DVS Support for DIS/DVS ver.1 (2-axes))
@@ -197,33 +195,15 @@ struct ia_css_dvs_stat_grid_info {
 
 /** DVS statistics generated by accelerator default grid info
  */
-#define DEFAULT_DVS_STAT_PUBLIC_DVS_GLOBAL_CFG \
-(struct dvs_stat_public_dvs_global_cfg) { }
-
-#define DEFAULT_DVS_STAT_PUBLIC_DVS_GRD_CFG \
-(struct dvs_stat_public_dvs_grd_cfg) { }
-
-#define DEFAULT_DVS_STAT_PUBLIC_DVS_LEVEL_FE_ROI_CFG(X_START) \
-   (struct dvs_stat_public_dvs_level_fe_roi_cfg) { .x_start = X_START, }
-
-#define DEFAULT_DVS_STAT_GRID_INFO \
-(struct ia_css_dvs_stat_grid_info) { \
-   .dvs_gbl_cfg = DEFAULT_DVS_STAT_PUBLIC_DVS_GLOBAL_CFG, \
-   .grd_cfg = { \
-   DEFAULT_DVS_STAT_PUBLIC_DVS_GRD_CFG, \
-   DEFAULT_DVS_STAT_PUBLIC_DVS_GRD_CFG, \
-   DEFAULT_DVS_STAT_PUBLIC_DVS_GRD_CFG \
-   }, \
-   .fe_roi_cfg = { \
-   DEFAULT_DVS_STAT_PUBLIC_DVS_LEVEL_FE_ROI_CFG(0), \
-   DEFAULT_DVS_STAT_PUBLIC_DVS_LEVEL_FE_ROI_CFG(4), \
-   DEFAULT_DVS_STAT_PUBLIC_DVS_LEVEL_FE_ROI_CFG(0), \
-   } \
-}
-
 #define DEFAULT_DVS_GRID_INFO \
 (union ia_css_dvs_grid_u) { \
-   .dvs_stat_grid_info = DEFAULT_DVS_STAT_GRID_INFO, \
+   .dvs_stat_grid_info = (struct ia_css_dvs_stat_grid_info) { \
+   .fe_roi_cfg = { \
+   [1] = (struct dvs_stat_public_dvs_level_fe_roi_cfg) 

[PATCH 0/3] Clean up of data-structure initialization in the CSS API

[Jeremy Sowden]

The CSS API uses a lot of nested anonymous structs defined in object
macros to assign default values to its data-structures.  These have been
changed to use compound-literals and designated initializers to make
them more comprehensible and less fragile.

The compound-literals can also be used in assignment, which made it
possible get rid of some temporary variables whose only purpose is to be
initialized by one of these anonymous structs and then serve as the
rvalue in an assignment expression.

The designated initializers also allow the removal of lots of
struct-members initialized to zero values.

I made the changes in three stages: firstly, I converted the default
values to compound-literals and designated initializers and removed the
temporary variables; secondly, I removed the zero-valued struct-members;
finally, I removed some structs which had become empty.

Jeremy Sowden (3):
  media: atomisp: convert default struct values to use compound-literals
with designated initializers.
  media: atomisp: delete zero-valued struct members.
  media: atomisp: delete empty default struct values.

 .../hive_isp_css_common/input_formatter_global.h   |  16 ---
 .../pci/atomisp2/css2400/ia_css_frame_public.h |  29 ++
 .../atomisp/pci/atomisp2/css2400/ia_css_pipe.h | 110 +++--
 .../pci/atomisp2/css2400/ia_css_pipe_public.h  | 108 +++-
 .../atomisp/pci/atomisp2/css2400/ia_css_types.h|  64 +++-
 .../isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h |  50 +-
 .../kernels/sdis/common/ia_css_sdis_common_types.h |  31 ++
 .../isp/kernels/sdis/sdis_1.0/ia_css_sdis.host.c   |   3 +-
 .../runtime/binary/interface/ia_css_binary.h   |  88 ++---
 .../atomisp2/css2400/runtime/binary/src/binary.c   |   3 +-
 .../isp_param/interface/ia_css_isp_param_types.h   |  10 --
 .../runtime/pipeline/interface/ia_css_pipeline.h   |  24 ++---
 .../css2400/runtime/pipeline/src/pipeline.c|   7 +-
 .../media/atomisp/pci/atomisp2/css2400/sh_css.c|  31 ++
 .../atomisp/pci/atomisp2/css2400/sh_css_legacy.h   |  11 ---
 .../atomisp/pci/atomisp2/css2400/sh_css_metrics.h  |  21 
 16 files changed, 114 insertions(+), 492 deletions(-)


base-commit: 37cb8e1f8e10c6e9bd2a1b95cdda0620a21b0551
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] media: atomisp: delete zero-valued struct members.

[Jeremy Sowden]

A lot of the members of the default struct values used by the CSS API
were explicitly initialized to zero values.  Designated initializers
have allowed these members to be removed.

Signed-off-by: Jeremy Sowden 
---
 .../hive_isp_css_common/input_formatter_global.h   |  17 
 .../pci/atomisp2/css2400/ia_css_frame_public.h |  17 
 .../atomisp/pci/atomisp2/css2400/ia_css_pipe.h |  42 +---
 .../pci/atomisp2/css2400/ia_css_pipe_public.h  |  82 ---
 .../atomisp/pci/atomisp2/css2400/ia_css_types.h|  47 +
 .../isp/kernels/s3a/s3a_1.0/ia_css_s3a_types.h | 113 +
 .../kernels/sdis/common/ia_css_sdis_common_types.h |  48 +
 .../runtime/binary/interface/ia_css_binary.h   |  90 ++--
 .../isp_param/interface/ia_css_isp_param_types.h   |  18 +---
 .../runtime/pipeline/interface/ia_css_pipeline.h   |   6 --
 .../atomisp/pci/atomisp2/css2400/sh_css_legacy.h   |  11 +-
 .../atomisp/pci/atomisp2/css2400/sh_css_metrics.h  |  11 +-
 12 files changed, 29 insertions(+), 473 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
index d5a586b08955..7558f4964313 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/input_formatter_global.h
@@ -107,23 +107,6 @@ struct input_formatter_cfg_s {
uint32_tblock_no_reqs;
 };
 
-#define DEFAULT_IF_CONFIG \
-(struct input_formatter_cfg_s) \
-{ \
-   .start_line = 0, \
-   .start_column   = 0, \
-   .left_padding   = 0, \
-   .cropped_height = 0, \
-   .cropped_width  = 0, \
-   .deinterleaving = 0, \
-   .buf_vecs   = 0, \
-   .buf_start_index= 0, \
-   .buf_increment  = 0, \
-   .buf_eol_offset = 0, \
-   .is_yuv420_format   = false, \
-   .block_no_reqs  = false \
-}
-
 extern const hrt_address HIVE_IF_SRST_ADDRESS[N_INPUT_FORMATTER_ID];
 extern const hrt_data HIVE_IF_SRST_MASK[N_INPUT_FORMATTER_ID];
 extern const uint8_t HIVE_IF_SWITCH_CODE[N_INPUT_FORMATTER_ID];
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h
index 786585037af9..b0872f93b3fa 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_frame_public.h
@@ -122,18 +122,8 @@ struct ia_css_frame_info {
 
 #define IA_CSS_BINARY_DEFAULT_FRAME_INFO \
 (struct ia_css_frame_info) { \
-   .res= (struct ia_css_resolution) { \
-   .width = 0, \
-   .height = 0 \
-   }, \
-   .padded_width   = 0, \
.format = IA_CSS_FRAME_FORMAT_NUM,  \
-   .raw_bit_depth  = 0, \
.raw_bayer_order= IA_CSS_BAYER_ORDER_NUM, \
-   .crop_info  = (struct ia_css_crop_info) { \
-   .start_column   = 0, \
-   .start_line = 0 \
-   }, \
 }
 
 /**
@@ -196,16 +186,9 @@ struct ia_css_frame {
 #define DEFAULT_FRAME \
 (struct ia_css_frame) { \
.info   = IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
-   .data   = 0, \
-   .data_bytes = 0, \
.dynamic_queue_id   = SH_CSS_INVALID_QUEUE_ID, \
.buf_type   = IA_CSS_BUFFER_TYPE_INVALID, \
.flash_state= IA_CSS_FRAME_FLASH_STATE_NONE, \
-   .exp_id = 0, \
-   .isp_config_id  = 0, \
-   .valid  = false, \
-   .contiguous = false, \
-   .planes = { 0 } \
 }
 
 /** @brief Fill a frame with zeros
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h
index 5d307679768e..a8f89866876d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe.h
@@ -44,11 +44,6 @@ struct ia_css_preview_settings {
.copy_binary= IA_CSS_BINARY_DEFAULT_SETTINGS, \
.preview_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
.vf_pp_binary   = IA_CSS_BINARY_DEFAULT_SETTINGS, \
-   .delay_frames   = { NULL }, \
-   .tnr_frames = { NULL }, \
-   .copy_pipe  = NULL, \
-   .capture_pipe   = NULL, \
-   .acc_pipe   = NULL, \
 }
 
 struct ia_css_capture_settings {
@@ -73,17 

[PATCH] staging: pi433: pi433_if.c Fix SET_CHECKED style issues

[Nguyen Phan Quang Minh]

Fix checkpatch warning and add result holder variable to reduce overhead
since the macro is called on functions.

Signed-off-by: Nguyen Phan Quang Minh 
---
Since SET_CHECKED has a return statement, I'm very tempted to straight
up remove it and do the checking inline. But the macro is used a lot
(~60 times), I think it might hurt the readability of the code.

 drivers/staging/pi433/pi433_if.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9722c9..77ab2986b985 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -122,9 +122,12 @@ struct pi433_instance {
 /*-*/
 
 /* macro for checked access of registers of radio module */
-#define SET_CHECKED(retval) \
-   if (retval < 0) \
-   return retval;
+#define SET_CHECKED(func)  \
+   do {\
+   int retval = func;  \
+   if (retval < 0) \
+   return retval;  \
+   } while (0)
 
 /*-*/
 
-- 
2.15.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/8] MIPS: Octeon: Enable LMTDMA/LMTST operations.

[James Hogan]

On Tue, Nov 28, 2017 at 04:55:34PM -0800, David Daney wrote:
> From: Carlos Munoz 
> 
> 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 
> Signed-off-by: Steven J. Hill 
> Signed-off-by: David Daney 
> ---
>  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 0x8100, 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 

Cheers
James


signature.asc
Description: Digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/5] mtd: nand: use usual return values for the ->erase() hook

[Boris Brezillon]

On Thu, 30 Nov 2017 18:01:28 +0100
Miquel Raynal  wrote:

> Avoid using specific defined values for checking returned status of the
> ->erase() hook. Instead, use usual negative error values on failure,  
> zero otherwise.
> 
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/mtd/nand/denali.c| 2 +-
>  drivers/mtd/nand/docg4.c | 7 ++-
>  drivers/mtd/nand/nand_base.c | 2 +-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 34008a02ddb0..3e19861a46c6 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -951,7 +951,7 @@ static int denali_erase(struct mtd_info *mtd, int page)
>   irq_status = denali_wait_for_irq(denali,
>INTR__ERASE_COMP | INTR__ERASE_FAIL);
>  
> - return irq_status & INTR__ERASE_COMP ? 0 : NAND_STATUS_FAIL;
> + return irq_status & INTR__ERASE_COMP ? 0 : -EIO;
>  }
>  
>  static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
> diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
> index 2436cbc71662..45c01b4b34c7 100644
> --- a/drivers/mtd/nand/docg4.c
> +++ b/drivers/mtd/nand/docg4.c
> @@ -900,6 +900,7 @@ static int docg4_erase_block(struct mtd_info *mtd, int 
> page)
>   struct docg4_priv *doc = nand_get_controller_data(nand);
>   void __iomem *docptr = doc->virtadr;
>   uint16_t g4_page;
> + int status;
>  
>   dev_dbg(doc->dev, "%s: page %04x\n", __func__, page);
>  
> @@ -939,7 +940,11 @@ static int docg4_erase_block(struct mtd_info *mtd, int 
> page)
>   poll_status(doc);
>   write_nop(docptr);
>  
> - return nand->waitfunc(mtd, nand);
> + status = nand->waitfunc(mtd, nand);
> + if (status < 0)
> + return status;
> +
> + return status & NAND_STATUS_FAIL ? -EIO : 0;
>  }
>  
>  static int write_page(struct mtd_info *mtd, struct nand_chip *nand,
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 630048f5abdc..4d1f2bda6095 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3077,7 +3077,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct 
> erase_info *instr,
>   status = chip->erase(mtd, page & chip->pagemask);
>  
>   /* See if block erase succeeded */
> - if (status & NAND_STATUS_FAIL) {
> + if (status) {
>   pr_debug("%s: failed erase, page 0x%08x\n",
>   __func__, page);
>   instr->state = MTD_ERASE_FAILED;

You forgot to patch single_erase() accordingly.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 5/5] mtd: nand: add ->exec_op() implementation

[Boris Brezillon]

On Thu, 30 Nov 2017 18:01:32 +0100
Miquel Raynal  wrote:

> Introduce a new interface to instruct NAND controllers to send specific
> NAND operations. The new interface takes the form of a single method
> called ->exec_op(). This method is designed to replace ->cmd_ctrl(),
> ->cmdfunc() and ->read/write_byte/word/buf() hooks.  
> 
> ->exec_op() is passed a set of instructions describing the operation  
> to execute. Each instruction has a type (ADDR, CMD, DATA, WAITRDY)
> and delay. The type is directly matching the description of NAND
> operations in various NAND datasheet and standards (ONFI, JEDEC), the
> delay is here to help simple controllers wait enough time between each
> instruction. Advanced controllers with integrated timings control can
> ignore these delays.
> 
> Advanced controllers (that are not limited to independent ADDR, CMD and
> DATA cycles) may use the parser added by this commit to get the best
> matching hook, if any. The instructions may be split by the parser in
> order to comply with the controller constraints filled in an array of
> supported patterns.
> 
> For instance, if a controller driver declares supporting up to 4 address
> cycles and then writes up to 512 bytes within one pattern (both are
> optional in this pattern):
> NAND_OP_PARSER_PAT_ADDR_ELEM(true, 4)
> NAND_OP_PARSER_PAT_DATA_OUT_ELEM(true, 512)
> It means that if the matching operation is made of 5 address cycles
> followed by 1024 bytes to write, then the controller will be asked to:
> - send 4 address cycles (the first four cycles),
> - send 1 address cycle (the last one) +
>   write 512 bytes (the first half),
> - write 512 bytes again (the second half).
> 
> Various other helpers are also added to ease NAND controller drivers
> writing.
> 
> This new interface should really ease the support of new vendor specific
> operations, and at least report whether the command is supported or not
> by a given controller, which was not possible before.
> 
> Suggested-by: Boris Brezillon 
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/mtd/nand/nand_base.c  | 1037 
> +++--
>  drivers/mtd/nand/nand_hynix.c |9 +
>  include/linux/mtd/rawnand.h   |  391 +++-
>  3 files changed, 1397 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 52965a8aeb2c..46bf31aff909 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -689,6 +689,59 @@ static void nand_wait_status_ready(struct mtd_info *mtd, 
> unsigned long timeo)
>  };
>  
>  /**
> + * nand_soft_waitrdy - Read the status waiting for it to be ready
> + * @chip: NAND chip structure
> + * @timeout_ms: Timeout in ms
> + *
> + * Poll the status using ->exec_op() until it is ready unless it takes too
> + * much time.
> + *
> + * This helper is intended to be used by drivers without R/B pin available to
> + * poll for the chip status until ready and may be called at any time in the
> + * middle of any set of instruction. The READ_STATUS just need to ask a 
> single
> + * time for it and then any read will return the status. Once the READ_STATUS
> + * cycles are done, the function will send a READ0 command to cancel the
> + * "READ_STATUS state" and let the normal flow of operation to continue.
> + *
> + * This helper *cannot* send a WAITRDY command or ->exec_op() implementations

  ^ instruction

> + * using it will enter an infinite loop.

Hm, not sure why this would be the case, but okay. Maybe you should
move this comment outside the kernel doc header, since this is an
implementation detail, not something the caller/user should be aware of.

There's another important aspect to mention here: this function can only
be called from an ->exec_op() implementation if this implementation is
re-entrant.

> + *
> + * Return 0 if the NAND chip is ready, a negative error otherwise.
> + */
> +int nand_soft_waitrdy(struct nand_chip *chip, unsigned long timeout_ms)
> +{
> + u8 status = 0;
> + int ret;
> +
> + if (!chip->exec_op)
> + return -ENOTSUPP;
> +
> + ret = nand_status_op(chip, NULL);
> + if (ret)
> + return ret;
> +
> + timeout_ms = jiffies + msecs_to_jiffies(timeout_ms);
> + do {
> + ret = nand_read_data_op(chip, , sizeof(status), true);
> + if (ret)
> + break;
> +
> + if (status & NAND_STATUS_READY)
> + break;
> +
> + udelay(100);

Sounds a bit high, especially for a read page which takes around 20us.

> + } while (time_before(jiffies, timeout_ms));
> +
> + nand_exit_status_op(chip);
> +
> + if (ret)
> + return ret;
> +
> + return status & NAND_STATUS_READY ? 0 : -ETIMEDOUT;
> +};
> 

[PATCH] staging: lustre: Fix sparse, using plain integer as NULL pointer in lov_object_fiemap()

[Andrii Vladyka]

Change 0 to NULL in lov_object_fiemap() in order to fix warning produced 
by sparse


Signed-off-by: Andrii Vladyka 


diff --git a/drivers/staging/lustre/lustre/lov/lov_object.c 
b/drivers/staging/lustre/lustre/lov/lov_object.c
index 105b707..897cf2c 100644
--- a/drivers/staging/lustre/lustre/lov/lov_object.c
+++ b/drivers/staging/lustre/lustre/lov/lov_object.c
@@ -1335,7 +1335,7 @@ static int lov_object_fiemap(const struct lu_env *env, 
struct cl_object *obj,
int rc = 0;
int cur_stripe;
int stripe_count;
-   struct fiemap_state fs = { 0 };
+   struct fiemap_state fs = { NULL };
 
lsm = lov_lsm_addref(cl2lov(obj));
if (!lsm)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode for stimer0

[Michael Kelley (EOSG)]

Thomas Gleixner  writes:

> On Tue, 31 Oct 2017, mikel...@exchange.microsoft.com wrote:
> > diff --git a/arch/x86/include/uapi/asm/hyperv.h
> > b/arch/x86/include/uapi/asm/hyperv.h
> > index f65d125..408cf3e 100644
> > --- a/arch/x86/include/uapi/asm/hyperv.h
> > +++ b/arch/x86/include/uapi/asm/hyperv.h
> > @@ -112,6 +112,22 @@
> >  #define HV_X64_GUEST_IDLE_STATE_AVAILABLE  (1 << 5)
> >  /* Guest crash data handler available */
> >  #define HV_X64_GUEST_CRASH_MSR_AVAILABLE   (1 << 10)
> > +/* Debug MSRs available */
> > +#define HV_X64_DEBUG_MSR_AVAILABLE (1 << 11)
> > +/* Support for Non-Privileged Instruction Execution Prevention is 
> > available */
> > +#define HV_X64_NPIEP_AVAILABLE (1 << 12)
> > +/* Support for DisableHypervisor is available */
> > +#define HV_X64_DISABLE_HYPERVISOR_AVAILABLE(1 << 13)
> > +/* Extended GVA Ranges for Flush Virtual Address list is available */
> > +#define HV_X64_EXTENDED_GVA_RANGE_AVAILABLE(1 << 14)
> > +/* Return Hypercall output via XMM registers is available */
> > +#define HV_X64_HYPERCALL_XMM_OUTPUT_AVAILABLE  (1 << 15)
> > +/* SINT polling mode available */
> > +#define HV_X64_SINT_POLLING_MODE_AVAILABLE (1 << 17)
> > +/* Hypercall MSR lock is available */
> > +#define HV_X64_HYPERCALL_MSR_LOCK_AVAILABLE(1 << 18)
> > +/* stimer direct mode is available */
> > +#define HV_X64_STIMER_DIRECT_MODE_AVAILABLE(1 << 19)
> 
> How are all these defines (except the last one related to that patch?

Will move to a separate patch.

> 
> > +/* Hardware IRQ number to use for stimer0 in Direct Mode.  This IRQ
> > +is a fake
> > + * because stimer's in Direct Mode simply interrupt on the specified
> > +vector,
> > + * without using a particular IOAPIC pin. But we use the IRQ
> > +allocation
> > + * machinery, so we need a hardware IRQ #.  This value is somewhat
> > +arbitrary,
> > + * but it should not be a legacy IRQ (0 to 15), and should fit within
> > +the
> > + * single IOAPIC (0 to 23) that Hyper-V provides to a guest VM. So
> > +any value
> > + * between 16 and 23 should be good.
> > + */
> > +#define HV_STIMER0_IRQNR   18
> 
> Why would you want abuse an IOAPIC interrupt if all you need is a vector?

Allocating a vector up-front like the existing HYPERVISOR_CALLBACK_VECTOR would 
certainly be more straightforward, and in fact, my original internal testing of 
stimer Direct Mode used that approach.   Vectors are a limited resource, so I 
wanted to find a way to allocate one on-the-fly rather than use fixed 
pre-allocation, even under CONFIG_HYPERV.   But I've got no problem with 
allocating a vector up-front and skipping all the irq/APIC manipulation and 
related issues.

Any guidance on which vector to use?

> 
> > +/* Routines to do per-architecture handling of stimer0 when in Direct
> > +Mode */
> > +
> > +void hv_ack_stimer0_interrupt(struct irq_desc *desc) {
> > +   ack_APIC_irq();
> > +}
> > +
> > +static void allonline_vector_allocation_domain(int cpu, struct cpumask 
> > *retmask,
> > +   const struct cpumask *mask)
> > +{
> > +   cpumask_copy(retmask, cpu_online_mask); }
> > +
> > +int hv_allocate_stimer0_irq(int *irq, int *vector) {
> > +   int localirq;
> > +   int result;
> > +   struct irq_data *irq_data;
> > +
> > +   /* The normal APIC vector allocation domain allows allocation of
> > +vectors
> 
> Please fix you comment style. Multi line comments are:
> 
>/*
> * Bla
>   * foo...
>   */
> 

Will do.

> > +* only for the calling CPU.  So we change the allocation domain to one
> > +* that allows vectors to be allocated in all online CPUs.  This
> > +* change is fine in a Hyper-V VM because VMs don't have the usual
> > +* complement of interrupting devices.
> > +*/
> > +   apic->vector_allocation_domain = allonline_vector_allocation_domain;
> 
> This does not work anymore. vector_allocation_domain is gone as of 4.15. 
> Please check the
> vector rework in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip x86/apic
> 
> Aside of that what guarantees that all cpus are online at the point where you 
> allocate that
> interrupt? Nothing, so the vector will be not reserved or allocated on 
> offline CPUs. Now guess
> what happens if you bring the offline CPUs online later, it will drown in 
> spurious interrupts.
> Worse, the vector can also be reused for a device interrupt. Great plan.
> 
> > +   localirq = acpi_register_gsi(NULL, HV_STIMER0_IRQNR,
> > +   ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
> > +   if (localirq < 0) {
> > +   pr_err("Cannot register stimer0 gsi. Error %d", localirq);
> > +   return -1;
> > +   }
> > +
> > +   /* We pass in a dummy IRQ handler because architecture independent code
> > +* will later override the IRQ domain 

RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode for stimer0

[Michael Kelley (EOSG)]

Vitaly Kuznetsov  writes:

> Vitaly Kuznetsov  writes:
> 
> > mikel...@exchange.microsoft.com writes:
> >
> >> From: Michael Kelley 
> >>
> >> The 2016 version of Hyper-V offers the option to operate the guest VM
> >> per-vcpu stimer's in Direct Mode, which means the timer interupts on
> >> its own vector rather than queueing a VMbus message.  Direct Mode
> >> reduces timer processing overhead in both the hypervisor and the
> >> guest, and avoids having timer interrupts pollute the VMbus interrupt
> >> stream for the synthetic NIC and storage.  This patch enables Direct
> >> Mode by default on stimer0 (which is the only stimer used in Linux on
> >> Hyper-V) when running on a version of Hyper-V that supports it, with
> >> a hv_vmbus module parameter for disabling Direct Mode and reverting
> >> to the old behavior.
> >>
> >> Signed-off-by: Michael Kelley 
> >> ---
> >>  arch/x86/include/asm/mshyperv.h| 14 
> >>  arch/x86/include/uapi/asm/hyperv.h | 26 ++
> >>  arch/x86/kernel/cpu/mshyperv.c | 64 +-
> >>  drivers/hv/hv.c| 71 
> >> --
> >>  drivers/hv/hyperv_vmbus.h  |  4 ++-
> >>  5 files changed, 175 insertions(+), 4 deletions(-)
> >>
> 
> (in addition to my previous comment)
> 
> This patch is also x86-heavy so it probably makes sense to make x86 
> maintainers (Thomas,
> Peter, Ingo) aware no matter which tree it will go through.
> 
> CC: x...@kernel.org
> 
> >> diff --git a/arch/x86/include/asm/mshyperv.h
> >> b/arch/x86/include/asm/mshyperv.h index 740dc97..1bba1d7 100644
> >> --- a/arch/x86/include/asm/mshyperv.h
> >> +++ b/arch/x86/include/asm/mshyperv.h
> >> @@ -4,6 +4,8 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>
> >> @@ -374,4 +376,16 @@ static inline struct ms_hyperv_tsc_page 
> >> *hv_get_tsc_page(void)
> >>return NULL;
> >>  }
> >>  #endif
> >> +
> >> +/* Per architecture routines for stimer0 Direct Mode handling.  On
> >> +x86/x64
> >> + * there are no percpu actions to take.
> >> + */
> >> +#if IS_ENABLED(CONFIG_HYPERV)
> >> +static inline void hv_enable_stimer0_percpu_irq(int irq) { } static
> >> +inline void hv_disable_stimer0_percpu_irq(int irq) { } extern int
> >> +hv_allocate_stimer0_irq(int *irq, int *vector); extern void
> >> +hv_deallocate_stimer0_irq(int irq); extern void
> >> +hv_ack_stimer0_interrupt(struct irq_desc *desc); #endif
> >> +
> >>  #endif
> >> diff --git a/arch/x86/include/uapi/asm/hyperv.h
> >> b/arch/x86/include/uapi/asm/hyperv.h
> >> index f65d125..408cf3e 100644
> >> --- a/arch/x86/include/uapi/asm/hyperv.h
> >> +++ b/arch/x86/include/uapi/asm/hyperv.h
> >> @@ -112,6 +112,22 @@
> >>  #define HV_X64_GUEST_IDLE_STATE_AVAILABLE (1 << 5)
> >>  /* Guest crash data handler available */
> >>  #define HV_X64_GUEST_CRASH_MSR_AVAILABLE  (1 << 10)
> >> +/* Debug MSRs available */
> >> +#define HV_X64_DEBUG_MSR_AVAILABLE(1 << 11)
> >> +/* Support for Non-Privileged Instruction Execution Prevention is 
> >> available */
> >> +#define HV_X64_NPIEP_AVAILABLE(1 << 12)
> >> +/* Support for DisableHypervisor is available */
> >> +#define HV_X64_DISABLE_HYPERVISOR_AVAILABLE   (1 << 13)
> >> +/* Extended GVA Ranges for Flush Virtual Address list is available */
> >> +#define HV_X64_EXTENDED_GVA_RANGE_AVAILABLE   (1 << 14)
> >> +/* Return Hypercall output via XMM registers is available */
> >> +#define HV_X64_HYPERCALL_XMM_OUTPUT_AVAILABLE (1 << 15)
> >> +/* SINT polling mode available */
> >> +#define HV_X64_SINT_POLLING_MODE_AVAILABLE(1 << 17)
> >> +/* Hypercall MSR lock is available */
> >> +#define HV_X64_HYPERCALL_MSR_LOCK_AVAILABLE   (1 << 18)
> >> +/* stimer direct mode is available */
> >> +#define HV_X64_STIMER_DIRECT_MODE_AVAILABLE   (1 << 19)
> >>
> >>  /*
> >>   * Implementation recommendations. Indicates which behaviors the
> >> hypervisor @@ -300,6 +316,16 @@ enum HV_GENERIC_SET_FORMAT {
> >>
> >>  #define HV_SYNIC_STIMER_COUNT (4)
> >>
> >> +/* Hardware IRQ number to use for stimer0 in Direct Mode.  This IRQ
> >> +is a fake
> >> + * because stimer's in Direct Mode simply interrupt on the specified
> >> +vector,
> >> + * without using a particular IOAPIC pin. But we use the IRQ
> >> +allocation
> >> + * machinery, so we need a hardware IRQ #.  This value is somewhat
> >> +arbitrary,
> >> + * but it should not be a legacy IRQ (0 to 15), and should fit
> >> +within the
> >> + * single IOAPIC (0 to 23) that Hyper-V provides to a guest VM. So
> >> +any value
> >> + * between 16 and 23 should be good.
> >> + */
> >
> > (nitpick): all comments in the patch are in 'net' style:
> >
> >  /* This is a
> >   * comment.
> >   */
> >
> > While 

[PATCH] staging: pi433: pi433_if.c added a blank in for loop

[Oliver Graute]

This patch fixes the following checkpatch.pl error:

ERROR: spaces required around that '=' (ctx:VxV)
#912: FILE: pi433_if.c:912:
+   for (i=0; i
---
 drivers/staging/pi433/pi433_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index c134cc3..74d4abe 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -890,7 +890,7 @@ static int setup_GPIOs(struct pi433_device *device)
DIO1_irq_handler
};
 
-   for (i=0; i

[PATCH] staging: pi433: pi433_if.c codestyle fix in IRQ_handler

[Oliver Graute]

This patch fixes the following checkpatch.pl errors:

ERROR: that open brace { should be on the previous line
#344: FILE: pi433_if.c:344:
+   if(retval) /* wait was interrupted */
+   {

ERROR: space required before the open parenthesis '('
#344: FILE: pi433_if.c:344:
+   if(retval) /* wait was interrupted */


Signed-off-by: Oliver Graute 
---
 drivers/staging/pi433/pi433_if.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 7632849..1cdff5c 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -327,8 +327,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
/* wait for any tx to finish */
dev_dbg(dev->dev, "rx: going to wait for any tx to finish");
retval = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active);
-   if(retval) /* wait was interrupted */
-   {
+   if (retval) /* wait was interrupted */ {
dev->interrupt_rx_allowed = true;
wake_up_interruptible(>tx_wait_queue);
return retval;
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: pi433: pi433_if.c codestyle fix missing blank

[Oliver Graute]

This patch fixes the following checkpatch.pl error:

ERROR: space required after that ',' (ctx:VxV)
#342: FILE: pi433_if.c:342:
+   dev_dbg(dev->dev,"rx: going to wait for any tx to finish");

Signed-off-by: Oliver Graute 
---
 drivers/staging/pi433/pi433_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 98eb25d..7632849 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -325,7 +325,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
dev->interrupt_rx_allowed = false;
 
/* wait for any tx to finish */
-   dev_dbg(dev->dev,"rx: going to wait for any tx to finish");
+   dev_dbg(dev->dev, "rx: going to wait for any tx to finish");
retval = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active);
if(retval) /* wait was interrupted */
{
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: pi433: Possible bug in rf69.c

[Marcus Wolf]

Hi Greg,

don't know, wether that's best option.

With that procedure, it will be very hard, to integrate large patches, 
if the owner of the patch isn't dealing with kernel source in his daily 
business and thus isn't able to react on new releases within no time.


I've seen the release of 4.15rc1 on Tuesday and already pulled the new 
head. But I am busy with my customer (proprietary software for an really 
ancient µC), so I won't be able to prepare my patches before weekend.


The patches will touch almost every function in rf69.c, since they 
change some basic concepts over there.


Cheers,

Marcus

Am 30.11.2017 um 19:12 schrieb Greg KH:

On Thu, Nov 30, 2017 at 06:01:46PM +0100, Marcin Ciupak wrote:

On Sat, Nov 11, 2017 at 01:51:10PM +0200, Marcus Wolf wrote:
Hi Marcus,

since 4.15-rc1 is out I would like to ask if you are going to provide
your changes anytime soon?

I would like to send a few patches as well and do not want to block your
work.


Just send patches, first one to my inbox always wins, don't wait for
someone else :)

greg k-h



--
Smarthome-Wolf UG (haftungsbeschränkt)
Helene-Lange-Weg 23
80637 München
Amtsgericht München, HRB 223529
Umastzsteuer-ID: DE304719911
Geschäftsführer: Marcus Wolf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH -next] staging: ipx: depends on NET

[Greg Kroah-Hartman]

On Thu, Nov 30, 2017 at 09:00:03AM -0800, Randy Dunlap wrote:
> From: Randy Dunlap 
> 
> IPX depends on NET, so add that to the Kconfig file.
> 
> Fixes Kconfig warning and build errors:
> 
> warning: (IPX) selects LLC which has unmet direct dependencies (NET)
> and 94 "undefined reference" build errors.
> 
> Signed-off-by: Randy Dunlap 
> ---
>  drivers/staging/ipx/Kconfig |1 +
>  1 file changed, 1 insertion(+)

Good catch, now queued up, thanks.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: pi433: Possible bug in rf69.c

[Greg KH]

On Thu, Nov 30, 2017 at 06:01:46PM +0100, Marcin Ciupak wrote:
> On Sat, Nov 11, 2017 at 01:51:10PM +0200, Marcus Wolf wrote:
> Hi Marcus,
> 
> since 4.15-rc1 is out I would like to ask if you are going to provide
> your changes anytime soon?
> 
> I would like to send a few patches as well and do not want to block your
> work.

Just send patches, first one to my inbox always wins, don't wait for
someone else :)

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/5] mtd: nand: force drivers to explicitly send READ/PROG commands

[Miquel Raynal]

From: Boris Brezillon 

The core currently send the READ0 and SEQIN+PAGEPROG commands in
nand_do_read/write_ops(). This is inconsistent with
->read/write_oob[_raw]() hooks behavior which are expected to send
these commands.

There's already a flag (NAND_ECC_CUSTOM_PAGE_ACCESS) to inform the core
that a specific controller wants to send the READ/SEQIN+PAGEPROG
commands on its own, but it's an opt-in flag, and existing drivers are
unlikely to be updated to pass it.

Moreover, some controllers cannot dissociate the READ/PAGEPROG commands
from the associated data transfer and ECC engine activation, and
developers have to hack things in their ->cmdfunc() implementation to
handle such complex cases, or have to accept the perf penalty of sending
twice the same command.
To address this problem we are planning on adding a new interface which
is passed all information about a NAND operation (including the amount
of data to transfer) and replacing all calls to ->cmdfunc() to calls to
this new ->exec_op() hook. But, in order to do that, we need to have all
->cmdfunc() calls placed near their associated ->read/write_buf/byte()
calls.

Modify the core and relevant drivers to make NAND_ECC_CUSTOM_PAGE_ACCESS
the default case, and remove this flag.

Signed-off-by: Boris Brezillon 
[miquel.ray...@free-electrons.com: tested, fixed and rebased on nand/next]
Signed-off-by: Miquel Raynal 
---
 drivers/mtd/nand/atmel/nand-controller.c  |  7 ++-
 drivers/mtd/nand/bf5xx_nand.c |  6 +-
 drivers/mtd/nand/brcmnand/brcmnand.c  | 13 +++-
 drivers/mtd/nand/cafe_nand.c  |  6 +-
 drivers/mtd/nand/denali.c |  1 -
 drivers/mtd/nand/docg4.c  | 12 ++--
 drivers/mtd/nand/fsl_elbc_nand.c  | 10 +--
 drivers/mtd/nand/fsl_ifc_nand.c   |  6 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c| 31 +-
 drivers/mtd/nand/hisi504_nand.c   |  6 +-
 drivers/mtd/nand/lpc32xx_mlc.c|  5 +-
 drivers/mtd/nand/lpc32xx_slc.c| 11 +++-
 drivers/mtd/nand/mtk_nand.c   | 22 +++
 drivers/mtd/nand/nand_base.c  | 87 +++
 drivers/mtd/nand/nand_micron.c| 56 ++---
 drivers/mtd/nand/omap2.c  | 10 ++-
 drivers/mtd/nand/pxa3xx_nand.c|  6 +-
 drivers/mtd/nand/qcom_nandc.c | 11 
 drivers/mtd/nand/sh_flctl.c   |  6 +-
 drivers/mtd/nand/sunxi_nand.c | 34 +++
 drivers/mtd/nand/tango_nand.c |  1 -
 drivers/mtd/nand/vf610_nfc.c  |  6 +-
 drivers/staging/mt29f_spinand/mt29f_spinand.c |  5 +-
 include/linux/mtd/rawnand.h   | 11 
 24 files changed, 171 insertions(+), 198 deletions(-)

diff --git a/drivers/mtd/nand/atmel/nand-controller.c 
b/drivers/mtd/nand/atmel/nand-controller.c
index e81fdd2d47b1..b2f00b398490 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -841,6 +841,8 @@ static int atmel_nand_pmecc_write_pg(struct nand_chip 
*chip, const u8 *buf,
struct atmel_nand *nand = to_atmel_nand(chip);
int ret;
 
+   nand_prog_page_begin_op(chip, page, 0, NULL, 0);
+
ret = atmel_nand_pmecc_enable(chip, NAND_ECC_WRITE, raw);
if (ret)
return ret;
@@ -857,7 +859,7 @@ static int atmel_nand_pmecc_write_pg(struct nand_chip 
*chip, const u8 *buf,
 
atmel_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
 
-   return 0;
+   return nand_prog_page_end_op(chip);
 }
 
 static int atmel_nand_pmecc_write_page(struct mtd_info *mtd,
@@ -881,6 +883,8 @@ static int atmel_nand_pmecc_read_pg(struct nand_chip *chip, 
u8 *buf,
struct mtd_info *mtd = nand_to_mtd(chip);
int ret;
 
+   nand_read_page_op(chip, page, 0, NULL, 0);
+
ret = atmel_nand_pmecc_enable(chip, NAND_ECC_READ, raw);
if (ret)
return ret;
@@ -1178,7 +1182,6 @@ static int atmel_hsmc_nand_ecc_init(struct atmel_nand 
*nand)
chip->ecc.write_page = atmel_hsmc_nand_pmecc_write_page;
chip->ecc.read_page_raw = atmel_hsmc_nand_pmecc_read_page_raw;
chip->ecc.write_page_raw = atmel_hsmc_nand_pmecc_write_page_raw;
-   chip->ecc.options |= NAND_ECC_CUSTOM_PAGE_ACCESS;
 
return 0;
 }
diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index 5655dca6ce43..87bbd177b3e5 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -572,6 +572,8 @@ static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
 static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip 
*chip,
uint8_t *buf, int oob_required, int page)
 {
+   nand_read_page_op(chip, page, 0, NULL, 0);
+

[PATCH 5/5] mtd: nand: add ->exec_op() implementation

[Miquel Raynal]

Introduce a new interface to instruct NAND controllers to send specific
NAND operations. The new interface takes the form of a single method
called ->exec_op(). This method is designed to replace ->cmd_ctrl(),
->cmdfunc() and ->read/write_byte/word/buf() hooks.

->exec_op() is passed a set of instructions describing the operation
to execute. Each instruction has a type (ADDR, CMD, DATA, WAITRDY)
and delay. The type is directly matching the description of NAND
operations in various NAND datasheet and standards (ONFI, JEDEC), the
delay is here to help simple controllers wait enough time between each
instruction. Advanced controllers with integrated timings control can
ignore these delays.

Advanced controllers (that are not limited to independent ADDR, CMD and
DATA cycles) may use the parser added by this commit to get the best
matching hook, if any. The instructions may be split by the parser in
order to comply with the controller constraints filled in an array of
supported patterns.

For instance, if a controller driver declares supporting up to 4 address
cycles and then writes up to 512 bytes within one pattern (both are
optional in this pattern):
NAND_OP_PARSER_PAT_ADDR_ELEM(true, 4)
NAND_OP_PARSER_PAT_DATA_OUT_ELEM(true, 512)
It means that if the matching operation is made of 5 address cycles
followed by 1024 bytes to write, then the controller will be asked to:
- send 4 address cycles (the first four cycles),
- send 1 address cycle (the last one) +
  write 512 bytes (the first half),
- write 512 bytes again (the second half).

Various other helpers are also added to ease NAND controller drivers
writing.

This new interface should really ease the support of new vendor specific
operations, and at least report whether the command is supported or not
by a given controller, which was not possible before.

Suggested-by: Boris Brezillon 
Signed-off-by: Miquel Raynal 
---
 drivers/mtd/nand/nand_base.c  | 1037 +++--
 drivers/mtd/nand/nand_hynix.c |9 +
 include/linux/mtd/rawnand.h   |  391 +++-
 3 files changed, 1397 insertions(+), 40 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 52965a8aeb2c..46bf31aff909 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -689,6 +689,59 @@ static void nand_wait_status_ready(struct mtd_info *mtd, 
unsigned long timeo)
 };
 
 /**
+ * nand_soft_waitrdy - Read the status waiting for it to be ready
+ * @chip: NAND chip structure
+ * @timeout_ms: Timeout in ms
+ *
+ * Poll the status using ->exec_op() until it is ready unless it takes too
+ * much time.
+ *
+ * This helper is intended to be used by drivers without R/B pin available to
+ * poll for the chip status until ready and may be called at any time in the
+ * middle of any set of instruction. The READ_STATUS just need to ask a single
+ * time for it and then any read will return the status. Once the READ_STATUS
+ * cycles are done, the function will send a READ0 command to cancel the
+ * "READ_STATUS state" and let the normal flow of operation to continue.
+ *
+ * This helper *cannot* send a WAITRDY command or ->exec_op() implementations
+ * using it will enter an infinite loop.
+ *
+ * Return 0 if the NAND chip is ready, a negative error otherwise.
+ */
+int nand_soft_waitrdy(struct nand_chip *chip, unsigned long timeout_ms)
+{
+   u8 status = 0;
+   int ret;
+
+   if (!chip->exec_op)
+   return -ENOTSUPP;
+
+   ret = nand_status_op(chip, NULL);
+   if (ret)
+   return ret;
+
+   timeout_ms = jiffies + msecs_to_jiffies(timeout_ms);
+   do {
+   ret = nand_read_data_op(chip, , sizeof(status), true);
+   if (ret)
+   break;
+
+   if (status & NAND_STATUS_READY)
+   break;
+
+   udelay(100);
+   } while (time_before(jiffies, timeout_ms));
+
+   nand_exit_status_op(chip);
+
+   if (ret)
+   return ret;
+
+   return status & NAND_STATUS_READY ? 0 : -ETIMEDOUT;
+};
+EXPORT_SYMBOL_GPL(nand_soft_waitrdy);
+
+/**
  * nand_command - [DEFAULT] Send command to NAND device
  * @mtd: MTD device structure
  * @command: the command to be sent
@@ -1238,6 +1291,134 @@ static int nand_init_data_interface(struct nand_chip 
*chip)
 }
 
 /**
+ * nand_fill_column_cycles - fill the column fields on an address array
+ * @chip: The NAND chip
+ * @addrs: Array of address cycles to fill
+ * @offset_in_page: The offset in the page
+ *
+ * Fills the first or the two first bytes of the @addrs field depending
+ * on the NAND bus width and the page size.
+ */
+static int nand_fill_column_cycles(struct nand_chip *chip, u8 *addrs,
+  unsigned int offset_in_page)
+{
+   struct mtd_info *mtd = nand_to_mtd(chip);
+
+  

[PATCH 2/5] mtd: nand: provide several helpers to do common NAND operations

[Miquel Raynal]

From: Boris Brezillon 

This is part of the process of removing direct calls to ->cmdfunc()
outside of the core in order to introduce a better interface to execute
NAND operations.

Here we provide several helpers and make use of them to remove all
direct calls to ->cmdfunc(). This way, we can easily modify those
helpers to make use of the new ->exec_op() interface when available.

Signed-off-by: Boris Brezillon 
[miquel.ray...@free-electrons.com: rebased and fixed some conflicts]
Signed-off-by: Miquel Raynal 
---
 drivers/mtd/nand/atmel/nand-controller.c |2 +-
 drivers/mtd/nand/brcmnand/brcmnand.c |9 +-
 drivers/mtd/nand/cafe_nand.c |   14 +-
 drivers/mtd/nand/denali.c|   37 +-
 drivers/mtd/nand/diskonchip.c|4 +-
 drivers/mtd/nand/docg4.c |2 +-
 drivers/mtd/nand/fsmc_nand.c |5 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c   |   58 +-
 drivers/mtd/nand/hisi504_nand.c  |3 +-
 drivers/mtd/nand/jz4740_nand.c   |   16 +-
 drivers/mtd/nand/lpc32xx_mlc.c   |2 +-
 drivers/mtd/nand/lpc32xx_slc.c   |   22 +-
 drivers/mtd/nand/mtk_nand.c  |   11 +-
 drivers/mtd/nand/nand_base.c | 1007 +-
 drivers/mtd/nand/nand_hynix.c|  115 ++--
 drivers/mtd/nand/nand_micron.c   |   77 ++-
 drivers/mtd/nand/omap2.c |8 +-
 drivers/mtd/nand/pxa3xx_nand.c   |8 +-
 drivers/mtd/nand/qcom_nandc.c|   16 +-
 drivers/mtd/nand/r852.c  |   11 +-
 drivers/mtd/nand/sunxi_nand.c|   71 +--
 drivers/mtd/nand/tango_nand.c|   26 +-
 drivers/mtd/nand/tmio_nand.c |5 +-
 include/linux/mtd/rawnand.h  |   27 +
 24 files changed, 1130 insertions(+), 426 deletions(-)

diff --git a/drivers/mtd/nand/atmel/nand-controller.c 
b/drivers/mtd/nand/atmel/nand-controller.c
index 90a71a56bc23..e81fdd2d47b1 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -1000,7 +1000,7 @@ static int atmel_hsmc_nand_pmecc_read_pg(struct nand_chip 
*chip, u8 *buf,
 * to the non-optimized one.
 */
if (nand->activecs->rb.type != ATMEL_NAND_NATIVE_RB) {
-   chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+   nand_read_page_op(chip, page, 0, NULL, 0);
 
return atmel_nand_pmecc_read_pg(chip, buf, oob_required, page,
raw);
diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c 
b/drivers/mtd/nand/brcmnand/brcmnand.c
index e0eb51d8c012..3f441096a14c 100644
--- a/drivers/mtd/nand/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/brcmnand/brcmnand.c
@@ -1071,7 +1071,7 @@ static void brcmnand_wp(struct mtd_info *mtd, int wp)
return;
 
brcmnand_set_wp(ctrl, wp);
-   chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
+   nand_status_op(chip, NULL);
/* NAND_STATUS_WP 0x00 = protected, 0x80 = not protected */
ret = bcmnand_ctrl_poll_status(ctrl,
   NAND_CTRL_RDY |
@@ -1453,7 +1453,7 @@ static uint8_t brcmnand_read_byte(struct mtd_info *mtd)
 
/* At FC_BYTES boundary, switch to next column */
if (host->last_byte > 0 && offs == 0)
-   chip->cmdfunc(mtd, NAND_CMD_RNDOUT, addr, -1);
+   nand_change_read_column_op(chip, addr, NULL, 0, false);
 
ret = ctrl->flash_cache[offs];
break;
@@ -1689,7 +1689,7 @@ static int brcmstb_nand_verify_erased_page(struct 
mtd_info *mtd,
sas = mtd->oobsize / chip->ecc.steps;
 
/* read without ecc for verification */
-   chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+   nand_read_page_op(chip, page, 0, NULL, 0);
ret = chip->ecc.read_page_raw(mtd, chip, buf, true, page);
if (ret)
return ret;
@@ -2369,12 +2369,11 @@ static int brcmnand_resume(struct device *dev)
 
list_for_each_entry(host, >host_list, node) {
struct nand_chip *chip = >chip;
-   struct mtd_info *mtd = nand_to_mtd(chip);
 
brcmnand_save_restore_cs_config(host, 1);
 
/* Reset the chip, required by some chips after power-up */
-   chip->cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+   nand_reset_op(chip);
}
 
return 0;
diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index bc558c438a57..95c2cfa68b66 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -353,23 +353,15 @@ static void cafe_nand_bug(struct mtd_info *mtd)
 static int cafe_nand_write_oob(struct mtd_info *mtd,
   struct 

[PATCH 4/5] mtd: nand: use a static data_interface in the nand_chip structure

[Miquel Raynal]

Change the nand_chip structure, to embed the nand_data_interface object.

Also remove the nand_get_default_data_interface() function that become
useless but add the initialization of the data_interface at the very
beginning of nand_scan_ident() to be sure core functions using timings
may be used safely.

Signed-off-by: Miquel Raynal 
---
 drivers/mtd/nand/nand_base.c| 44 ++---
 drivers/mtd/nand/nand_timings.c | 21 +---
 include/linux/mtd/rawnand.h |  7 ++-
 3 files changed, 26 insertions(+), 46 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ee9825780a6c..52965a8aeb2c 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -816,8 +816,8 @@ static void nand_ccs_delay(struct nand_chip *chip)
 * Wait tCCS_min if it is correctly defined, otherwise wait 500ns
 * (which should be safe for all NANDs).
 */
-   if (chip->data_interface && chip->data_interface->timings.sdr.tCCS_min)
-   ndelay(chip->data_interface->timings.sdr.tCCS_min / 1000);
+   if (chip->setup_data_interface)
+   ndelay(chip->data_interface.timings.sdr.tCCS_min / 1000);
else
ndelay(500);
 }
@@ -1112,7 +1112,6 @@ static int nand_wait(struct mtd_info *mtd, struct 
nand_chip *chip)
 static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
 {
struct mtd_info *mtd = nand_to_mtd(chip);
-   const struct nand_data_interface *conf;
int ret;
 
if (!chip->setup_data_interface)
@@ -1132,8 +1131,8 @@ static int nand_reset_data_interface(struct nand_chip 
*chip, int chipnr)
 * timings to timing mode 0.
 */
 
-   conf = nand_get_default_data_interface();
-   ret = chip->setup_data_interface(mtd, chipnr, conf);
+   onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
+   ret = chip->setup_data_interface(mtd, chipnr, >data_interface);
if (ret)
pr_err("Failed to configure data interface to SDR timing mode 
0\n");
 
@@ -1158,7 +1157,7 @@ static int nand_setup_data_interface(struct nand_chip 
*chip, int chipnr)
struct mtd_info *mtd = nand_to_mtd(chip);
int ret;
 
-   if (!chip->setup_data_interface || !chip->data_interface)
+   if (!chip->setup_data_interface)
return 0;
 
/*
@@ -1179,7 +1178,7 @@ static int nand_setup_data_interface(struct nand_chip 
*chip, int chipnr)
goto err;
}
 
-   ret = chip->setup_data_interface(mtd, chipnr, chip->data_interface);
+   ret = chip->setup_data_interface(mtd, chipnr, >data_interface);
 err:
return ret;
 }
@@ -1219,21 +1218,16 @@ static int nand_init_data_interface(struct nand_chip 
*chip)
modes = GENMASK(chip->onfi_timing_mode_default, 0);
}
 
-   chip->data_interface = kzalloc(sizeof(*chip->data_interface),
-  GFP_KERNEL);
-   if (!chip->data_interface)
-   return -ENOMEM;
 
for (mode = fls(modes) - 1; mode >= 0; mode--) {
-   ret = onfi_init_data_interface(chip, chip->data_interface,
-  NAND_SDR_IFACE, mode);
+   ret = onfi_fill_data_interface(chip, NAND_SDR_IFACE, mode);
if (ret)
continue;
 
/* Pass -1 to only */
ret = chip->setup_data_interface(mtd,
 NAND_DATA_IFACE_CHECK_ONLY,
-chip->data_interface);
+>data_interface);
if (!ret) {
chip->onfi_timing_mode_default = mode;
break;
@@ -1243,11 +1237,6 @@ static int nand_init_data_interface(struct nand_chip 
*chip)
return 0;
 }
 
-static void nand_release_data_interface(struct nand_chip *chip)
-{
-   kfree(chip->data_interface);
-}
-
 /**
  * nand_read_page_op - Do a READ PAGE operation
  * @chip: The NAND chip
@@ -1763,11 +1752,16 @@ EXPORT_SYMBOL_GPL(nand_write_data_op);
  * @chip: The NAND chip
  * @chipnr: Internal die id
  *
+ * Save the timings data structure, then apply SDR timings mode 0 (see
+ * nand_reset_data_interface for details), do the reset operation, and
+ * apply back the previous timings.
+ *
  * Returns 0 for success or negative error code otherwise
  */
 int nand_reset(struct nand_chip *chip, int chipnr)
 {
struct mtd_info *mtd = nand_to_mtd(chip);
+   struct nand_data_interface saved_data_intf = chip->data_interface;
int ret;
 
ret = nand_reset_data_interface(chip, chipnr);
@@ -1785,6 +1779,7 @@ int nand_reset(struct nand_chip *chip, int chipnr)
return ret;
 
chip->select_chip(mtd, chipnr);
+   chip->data_interface = 

Re: staging: pi433: Possible bug in rf69.c

[Marcin Ciupak]

On Sat, Nov 11, 2017 at 01:51:10PM +0200, Marcus Wolf wrote:
Hi Marcus,

since 4.15-rc1 is out I would like to ask if you are going to provide
your changes anytime soon?

I would like to send a few patches as well and do not want to block your
work.

Thanks,
Marcin

> Hi Greg,
> 
> ok.
> 
> I'll postpone all my work until then. Give me a hook, when I can start :-)
> 
> Thanks,
> 
> Marcus
> 
> 
> Am 11.11.2017 um 13:49 schrieb Greg Kroah-Hartman:
> > On Sat, Nov 11, 2017 at 01:42:27PM +0200, Marcus Wolf wrote:
> >> Hi Greg,
> >>
> >> that's fine.
> >>
> >> Is this the right URL:
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> > 
> > Yes.
> > 
> >> Is there already an aprox. date, when 4.15rc1 will be out and
> >> backintegration will be done?
> > 
> > Should be 2 weeks from now.
> > 
> > thanks,
> > 
> > greg k-h
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/5] Introduce the new NAND core interface: ->exec_op()

[Miquel Raynal]

Hi,

This series adds the implementation of the NAND framework ->exec_op()
interface with all the related hooks and helpers. The reasons for adding
it are explained in details in the commit log:

"mtd: nand: add ->exec_op() implementation"

Long story short: it will ease later expansion of the framework, as well
as the implementation of new vendor specific commands, and should also
ease driver development.

A lot of comments are written to explain how to use the new API. Several
NAND controller drivers are already/almost converted to ->exec_op(), in
particular a rework of the Marvell NAND controller driver, and will
follow. One can have a look at them as examples to understand how to
implement or rework NAND controller drivers. A proper external
documentation is being written and will later be submitted.

Thank you,
Miquèl


Boris Brezillon (2):
  mtd: nand: provide several helpers to do common NAND operations
  mtd: nand: force drivers to explicitly send READ/PROG commands

Miquel Raynal (3):
  mtd: nand: use usual return values for the ->erase() hook
  mtd: nand: use a static data_interface in the nand_chip structure
  mtd: nand: add ->exec_op() implementation

 drivers/mtd/nand/atmel/nand-controller.c  |9 +-
 drivers/mtd/nand/bf5xx_nand.c |6 +-
 drivers/mtd/nand/brcmnand/brcmnand.c  |   20 +-
 drivers/mtd/nand/cafe_nand.c  |   20 +-
 drivers/mtd/nand/denali.c |   40 +-
 drivers/mtd/nand/diskonchip.c |4 +-
 drivers/mtd/nand/docg4.c  |   21 +-
 drivers/mtd/nand/fsl_elbc_nand.c  |   10 +-
 drivers/mtd/nand/fsl_ifc_nand.c   |6 +-
 drivers/mtd/nand/fsmc_nand.c  |5 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c|   77 +-
 drivers/mtd/nand/hisi504_nand.c   |9 +-
 drivers/mtd/nand/jz4740_nand.c|   16 +-
 drivers/mtd/nand/lpc32xx_mlc.c|7 +-
 drivers/mtd/nand/lpc32xx_slc.c|   33 +-
 drivers/mtd/nand/mtk_nand.c   |   25 +-
 drivers/mtd/nand/nand_base.c  | 2161 ++---
 drivers/mtd/nand/nand_hynix.c |  124 +-
 drivers/mtd/nand/nand_micron.c|   83 +-
 drivers/mtd/nand/nand_timings.c   |   21 +-
 drivers/mtd/nand/omap2.c  |   18 +-
 drivers/mtd/nand/pxa3xx_nand.c|   14 +-
 drivers/mtd/nand/qcom_nandc.c |   27 +-
 drivers/mtd/nand/r852.c   |   11 +-
 drivers/mtd/nand/sh_flctl.c   |6 +-
 drivers/mtd/nand/sunxi_nand.c |   97 +-
 drivers/mtd/nand/tango_nand.c |   27 +-
 drivers/mtd/nand/tmio_nand.c  |5 +-
 drivers/mtd/nand/vf610_nfc.c  |6 +-
 drivers/staging/mt29f_spinand/mt29f_spinand.c |5 +-
 include/linux/mtd/rawnand.h   |  414 -
 31 files changed, 2673 insertions(+), 654 deletions(-)

-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/5] mtd: nand: use usual return values for the ->erase() hook

[Miquel Raynal]

Avoid using specific defined values for checking returned status of the
->erase() hook. Instead, use usual negative error values on failure,
zero otherwise.

Signed-off-by: Miquel Raynal 
---
 drivers/mtd/nand/denali.c| 2 +-
 drivers/mtd/nand/docg4.c | 7 ++-
 drivers/mtd/nand/nand_base.c | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 34008a02ddb0..3e19861a46c6 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -951,7 +951,7 @@ static int denali_erase(struct mtd_info *mtd, int page)
irq_status = denali_wait_for_irq(denali,
 INTR__ERASE_COMP | INTR__ERASE_FAIL);
 
-   return irq_status & INTR__ERASE_COMP ? 0 : NAND_STATUS_FAIL;
+   return irq_status & INTR__ERASE_COMP ? 0 : -EIO;
 }
 
 static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index 2436cbc71662..45c01b4b34c7 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -900,6 +900,7 @@ static int docg4_erase_block(struct mtd_info *mtd, int page)
struct docg4_priv *doc = nand_get_controller_data(nand);
void __iomem *docptr = doc->virtadr;
uint16_t g4_page;
+   int status;
 
dev_dbg(doc->dev, "%s: page %04x\n", __func__, page);
 
@@ -939,7 +940,11 @@ static int docg4_erase_block(struct mtd_info *mtd, int 
page)
poll_status(doc);
write_nop(docptr);
 
-   return nand->waitfunc(mtd, nand);
+   status = nand->waitfunc(mtd, nand);
+   if (status < 0)
+   return status;
+
+   return status & NAND_STATUS_FAIL ? -EIO : 0;
 }
 
 static int write_page(struct mtd_info *mtd, struct nand_chip *nand,
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 630048f5abdc..4d1f2bda6095 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3077,7 +3077,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct 
erase_info *instr,
status = chip->erase(mtd, page & chip->pagemask);
 
/* See if block erase succeeded */
-   if (status & NAND_STATUS_FAIL) {
+   if (status) {
pr_debug("%s: failed erase, page 0x%08x\n",
__func__, page);
instr->state = MTD_ERASE_FAILED;
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH -next] staging: ipx: depends on NET

[Randy Dunlap]

From: Randy Dunlap <rdun...@infradead.org>

IPX depends on NET, so add that to the Kconfig file.

Fixes Kconfig warning and build errors:

warning: (IPX) selects LLC which has unmet direct dependencies (NET)
and 94 "undefined reference" build errors.

Signed-off-by: Randy Dunlap <rdun...@infradead.org>
---
 drivers/staging/ipx/Kconfig |1 +
 1 file changed, 1 insertion(+)

--- linux-next-20171130.orig/drivers/staging/ipx/Kconfig
+++ linux-next-20171130/drivers/staging/ipx/Kconfig
@@ -3,6 +3,7 @@
 #
 config IPX
tristate "The IPX protocol"
+   depends on NET
select LLC
---help---
  This is support for the Novell networking protocol, IPX, commonly


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: irda: Handle return value of platform_get_irq

[arvindY]

Hi Greg,

On Thursday 30 November 2017 10:11 PM, Greg KH wrote:

On Thu, Nov 30, 2017 at 09:13:35PM +0530, Arvind Yadav wrote:

platform_get_irq() can fail here and we must check its return value.

Signed-off-by: Arvind Yadav 
---
  drivers/staging/irda/drivers/pxaficp_ir.c | 10 ++
  1 file changed, 10 insertions(+)

Did you read drivers/staging/irda/TODO?

Sorry, Now I have read it. :(


thanks,

greg k-h

Thanks,
~arvind
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/3] staging: irda: Handle return value of platform_get_irq

[Greg KH]

On Thu, Nov 30, 2017 at 09:13:35PM +0530, Arvind Yadav wrote:
> platform_get_irq() can fail here and we must check its return value.
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/staging/irda/drivers/pxaficp_ir.c | 10 ++
>  1 file changed, 10 insertions(+)

Did you read drivers/staging/irda/TODO?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/3] Handle return value of platform_get_irq

[Arvind Yadav]

The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct.

Remove unnecessary 'err' initialization for irda driver.

Arvind Yadav (3):
  [PATCH 1/3] iio: trigger: Fix platform_get_irq's error checking
  [PATCH 2/3] staging: irda: Handle return value of platform_get_irq
  [PATCH 3/3] staging: irda: Remove unnecessary 'err' initialization.

 drivers/staging/iio/trigger/iio-trig-bfin-timer.c |  4 ++--
 drivers/staging/irda/drivers/pxaficp_ir.c | 11 ++-
 2 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: irda: Remove unnecessary 'err' initialization.

[Arvind Yadav]

Here, variable 'err' is already initialised. So no need to reinitialize.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/irda/drivers/pxaficp_ir.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/irda/drivers/pxaficp_ir.c 
b/drivers/staging/irda/drivers/pxaficp_ir.c
index a97ce04..6c77370 100644
--- a/drivers/staging/irda/drivers/pxaficp_ir.c
+++ b/drivers/staging/irda/drivers/pxaficp_ir.c
@@ -807,7 +807,6 @@ static int pxa_irda_start(struct net_device *dev)
 * Open a new IrLAP layer instance.
 */
si->irlap = irlap_open(dev, >qos, "pxa");
-   err = -ENOMEM;
if (!si->irlap)
goto err_irlap;
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] iio: trigger: Fix platform_get_irq's error checking

[Arvind Yadav]

The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c 
b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
index d80dcf8..f389f5c 100644
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
@@ -187,9 +187,9 @@ static int iio_bfin_tmr_trigger_probe(struct 
platform_device *pdev)
return -ENOMEM;
 
st->irq = platform_get_irq(pdev, 0);
-   if (!st->irq) {
+   if (st->irq < 0) {
dev_err(>dev, "No IRQs specified");
-   return -ENODEV;
+   return st->irq;
}
 
ret = iio_bfin_tmr_get_number(st->irq);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: irda: Handle return value of platform_get_irq

[Arvind Yadav]

platform_get_irq() can fail here and we must check its return value.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/irda/drivers/pxaficp_ir.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/irda/drivers/pxaficp_ir.c 
b/drivers/staging/irda/drivers/pxaficp_ir.c
index 1dba16b..a97ce04 100644
--- a/drivers/staging/irda/drivers/pxaficp_ir.c
+++ b/drivers/staging/irda/drivers/pxaficp_ir.c
@@ -960,7 +960,17 @@ static int pxa_irda_probe(struct platform_device *pdev)
si->irda_base = ficp;
si->stuart_base = stuart;
si->uart_irq = platform_get_irq(pdev, 0);
+   if (si->uart_irq < 0) {
+   err =  si->uart_irq;
+   goto err_mem_1;
+   }
+
si->icp_irq = platform_get_irq(pdev, 1);
+   if (si->icp_irq < 0) {
+   err =  si->icp_irq;
+   goto err_mem_1;
+   }
+
 
si->sir_clk = devm_clk_get(>dev, "UARTCLK");
si->fir_clk = devm_clk_get(>dev, "FICPCLK");
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: xgifb: remove macros with hidden variable

[Joshua Abraham]

This patch removes macros in XGI_main.h that contain a xgifb_info
variable.  These macros hurt readability by hiding said variable
behind a define.

Signed-off-by: Joshua Abraham 
---
 drivers/staging/xgifb/XGI_main.h|  13 ---
 drivers/staging/xgifb/XGI_main_26.c | 227 +---
 2 files changed, 131 insertions(+), 109 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main.h b/drivers/staging/xgifb/XGI_main.h
index a3af1cbbf8ee..e19a8291cb2a 100644
--- a/drivers/staging/xgifb/XGI_main.h
+++ b/drivers/staging/xgifb/XGI_main.h
@@ -18,19 +18,6 @@ static const struct pci_device_id xgifb_pci_table[] = {
 
 MODULE_DEVICE_TABLE(pci, xgifb_pci_table);
 
-/* To be included in fb.h */
-#define XGISR(xgifb_info->dev_info.P3c4)
-#define XGICR(xgifb_info->dev_info.P3d4)
-#define XGIDACA  (xgifb_info->dev_info.P3c8)
-#define XGIDACD  (xgifb_info->dev_info.P3c9)
-#define XGIPART1 (xgifb_info->dev_info.Part1Port)
-#define XGIPART2 (xgifb_info->dev_info.Part2Port)
-#define XGIPART3 (xgifb_info->dev_info.Part3Port)
-#define XGIPART4 (xgifb_info->dev_info.Part4Port)
-#define XGIPART5 (xgifb_info->dev_info.Part5Port)
-#define XGIDAC2A  XGIPART5
-#define XGIDAC2D  (XGIPART5 + 1)
-
 #define IND_XGI_SCRATCH_REG_CR30  0x30  /* CRs */
 #define IND_XGI_SCRATCH_REG_CR31  0x31
 #define IND_XGI_SCRATCH_REG_CR32  0x32
diff --git a/drivers/staging/xgifb/XGI_main_26.c 
b/drivers/staging/xgifb/XGI_main_26.c
index 6feecc55d2bc..6de66eaad96b 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -34,16 +34,16 @@ static void dumpVGAReg(struct xgifb_video_info *xgifb_info)
 {
u8 i, reg;
 
-   xgifb_reg_set(XGISR, 0x05, 0x86);
+   xgifb_reg_set(xgifb_info->dev_info.P3c4, 0x05, 0x86);
 
for (i = 0; i < 0x4f; i++) {
-   reg = xgifb_reg_get(XGISR, i);
+   reg = xgifb_reg_get(xgifb_info->dev_info.P3c4, i);
pr_debug("o 3c4 %x\n", i);
pr_debug("i 3c5 => %x\n", reg);
}
 
for (i = 0; i < 0xF0; i++) {
-   reg = xgifb_reg_get(XGICR, i);
+   reg = xgifb_reg_get(xgifb_info->dev_info.P3d4, i);
pr_debug("o 3d4 %x\n", i);
pr_debug("i 3d5 => %x\n", reg);
}
@@ -647,7 +647,7 @@ static void XGIfb_pre_setmode(struct xgifb_video_info 
*xgifb_info)
 {
u8 cr30 = 0, cr31 = 0;
 
-   cr31 = xgifb_reg_get(XGICR, 0x31);
+   cr31 = xgifb_reg_get(xgifb_info->dev_info.P3d4, 0x31);
cr31 &= ~0x60;
 
switch (xgifb_info->display2) {
@@ -684,9 +684,12 @@ static void XGIfb_pre_setmode(struct xgifb_video_info 
*xgifb_info)
cr31 |= (SIS_DRIVER_MODE | SIS_VB_OUTPUT_DISABLE);
}
 
-   xgifb_reg_set(XGICR, IND_XGI_SCRATCH_REG_CR30, cr30);
-   xgifb_reg_set(XGICR, IND_XGI_SCRATCH_REG_CR31, cr31);
-   xgifb_reg_set(XGICR, IND_XGI_SCRATCH_REG_CR33,
+   xgifb_reg_set(xgifb_info->dev_info.P3d4,
+ IND_XGI_SCRATCH_REG_CR30, cr30);
+   xgifb_reg_set(xgifb_info->dev_info.P3d4,
+ IND_XGI_SCRATCH_REG_CR31, cr31);
+   xgifb_reg_set(xgifb_info->dev_info.P3d4,
+ IND_XGI_SCRATCH_REG_CR33,
  (xgifb_info->rate_idx & 0x0F));
 }
 
@@ -714,7 +717,7 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
*xgifb_info)
 
/* We can't switch off CRT1 if bridge is in slave mode */
if (xgifb_info->hasVB != HASVB_NONE) {
-   reg = xgifb_reg_get(XGIPART1, 0x00);
+   reg = xgifb_reg_get(xgifb_info->dev_info.Part1Port, 0x00);
 
if ((reg & 0x50) == 0x10)
doit = 0;
@@ -723,18 +726,18 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
*xgifb_info)
XGIfb_crt1off = 0;
}
 
-   reg = xgifb_reg_get(XGICR, 0x17);
+   reg = xgifb_reg_get(xgifb_info->dev_info.P3d4, 0x17);
if ((XGIfb_crt1off) && (doit))
reg &= ~0x80;
else
reg |= 0x80;
-   xgifb_reg_set(XGICR, 0x17, reg);
+   xgifb_reg_set(xgifb_info->dev_info.P3d4, 0x17, reg);
 
-   xgifb_reg_and(XGISR, IND_SIS_RAMDAC_CONTROL, ~0x04);
+   xgifb_reg_and(xgifb_info->dev_info.P3c4, IND_SIS_RAMDAC_CONTROL, ~0x04);
 
if (xgifb_info->display2 == XGIFB_DISP_TV &&
xgifb_info->hasVB == HASVB_301) {
-   reg = xgifb_reg_get(XGIPART4, 0x01);
+   reg = xgifb_reg_get(xgifb_info->dev_info.Part4Port, 0x01);
 
if (reg < 0xB0) { /* Set filter for XGI301 */
int filter_tb;
@@ -761,60 +764,68 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
*xgifb_info)
filter = -1;
   

Re: [PATCH] staging: xgifb: remove unused macro XGIPART3

[Josh Abraham]

On Thu, Nov 30, 2017 at 08:55:44AM +0300, Dan Carpenter wrote:
> On Wed, Nov 29, 2017 at 09:53:48PM -0500, Joshua Abraham wrote:
> > Signed-off-by: Joshua Abraham 
> > 
> > This patch removes the unused macro XGIPART3.
> > 
> 
> The Signed-off-by line goes after the changelog.
> 
> > ---
> >  drivers/staging/xgifb/XGI_main.h | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/staging/xgifb/XGI_main.h 
> > b/drivers/staging/xgifb/XGI_main.h
> > index a3af1cbbf8ee..5f55d0a39bc1 100644
> > --- a/drivers/staging/xgifb/XGI_main.h
> > +++ b/drivers/staging/xgifb/XGI_main.h
> > @@ -25,7 +25,6 @@ MODULE_DEVICE_TABLE(pci, xgifb_pci_table);
> >  #define XGIDACD  (xgifb_info->dev_info.P3c9)
> >  #define XGIPART1 (xgifb_info->dev_info.Part1Port)
> >  #define XGIPART2 (xgifb_info->dev_info.Part2Port)
> > -#define XGIPART3 (xgifb_info->dev_info.Part3Port)
> 
> That define isn't hurting anyone.
> 
> >  #define XGIPART4 (xgifb_info->dev_info.Part4Port)
> >  #define XGIPART5 (xgifb_info->dev_info.Part5Port)
> 
> Actually these should all be deleted because they mean you have to have
> a xgifb_info variable and they hurt readability by hiding stuff behind a
> define.  It would be better to remove them all than to just remove one
> from the middle.  That's a more complicated patch, but it's a useful
> patch.
> 
> regards,
> dan carpenter
> 

Great point.  I will work on that and get the patch out!

-Josh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel