Re: [PATCH] Fixed a minor coding style warning. Arguments in the macros should be coverd in brackets to aviod any precedence issues.

2017-03-13 Thread Greg KH
On Mon, Mar 13, 2017 at 09:52:14PM -0700, mshan wrote:
> Signed-off-by: mshan 
> ---
>  drivers/staging/fwserial/fwserial.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH v3 3/4] staging: sm750fb: Alignment should match open parenthesis

2017-03-13 Thread Julia Lawall


On Tue, 14 Mar 2017, Arushi Singhal wrote:

> Fix checkpatch issues: "CHECK: Alignment should match open parenthesis".

I thought you were going to take another approach to improve this code?

julia

>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/sm750fb/ddk750_mode.c | 79 
> +--
>  1 file changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
> b/drivers/staging/sm750fb/ddk750_mode.c
> index 45af806c8d55..eea5aef2956f 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -28,9 +28,9 @@ static unsigned long 
> displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
>   poke32(CRT_AUTO_CENTERING_TL, 0);
>
>   poke32(CRT_AUTO_CENTERING_BR,
> - (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
> - CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
> - ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
> +(((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
> + CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
> +((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
>
>   /*
>* Assume common fields in dispControl have been properly set before
> @@ -72,8 +72,7 @@ static unsigned long 
> displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
>  }
>
>  /* only timing related registers will be  programed */
> -static int programModeRegisters(mode_parameter_t *pModeParam,
> - struct pll_value *pll)
> +static int programModeRegisters(mode_parameter_t *pModeParam, struct 
> pll_value *pll)
>  {
>   int ret = 0;
>   int cnt = 0;
> @@ -83,32 +82,32 @@ static int programModeRegisters(mode_parameter_t 
> *pModeParam,
>   /* programe secondary pixel clock */
>   poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
>   poke32(CRT_HORIZONTAL_TOTAL,
> - (((pModeParam->horizontal_total - 1) <<
> - CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
> - CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
> - ((pModeParam->horizontal_display_end - 1) &
> - CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
> +(((pModeParam->horizontal_total - 1) <<
> +  CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
> + CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
> +((pModeParam->horizontal_display_end - 1) &
> + CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
>
>   poke32(CRT_HORIZONTAL_SYNC,
> - ((pModeParam->horizontal_sync_width <<
> - CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
> - CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
> - ((pModeParam->horizontal_sync_start - 1) &
> - CRT_HORIZONTAL_SYNC_START_MASK));
> +((pModeParam->horizontal_sync_width <<
> +  CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
> + CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
> +((pModeParam->horizontal_sync_start - 1) &
> + CRT_HORIZONTAL_SYNC_START_MASK));
>
>   poke32(CRT_VERTICAL_TOTAL,
> - (((pModeParam->vertical_total - 1) <<
> - CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
> - CRT_VERTICAL_TOTAL_TOTAL_MASK) |
> - ((pModeParam->vertical_display_end - 1) &
> - CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
> +(((pModeParam->vertical_total - 1) <<
> +  CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
> + CRT_VERTICAL_TOTAL_TOTAL_MASK) |
> +((pModeParam->vertical_display_end - 1) &
> + CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
>
>   poke32(CRT_VERTICAL_SYNC,
> - ((pModeParam->vertical_sync_height <<
> - CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
> - CRT_VERTICAL_SYNC_HEIGHT_MASK) |
> - ((pModeParam->vertical_sync_start - 1) &
> - CRT_VERTICAL_SYNC_START_MASK));
> +((pModeParam->vertical_sync_height <<
> +  CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
> + CRT_VERTICAL_SYNC_HEIGHT_MASK) |
> +((pModeParam->vertical_sync_start - 1) &
> + CRT_VERTICAL_SYNC_START_MASK));
>
>   tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
>   if (pModeParam->vertical_sync_polarity)
> @@ -140,25 +139,25 @@ static int programModeRegisters(mode_parameter_t 
> *pModeParam,
>   poke32(PANEL_HORIZONTAL_TOTAL, reg);
>
>   poke32(PANEL_HORIZONTAL_SYNC,
> - 

[PATCH] ks7010: adding parenthesis to macro argument

2017-03-13 Thread Pushkar Jambhlekar
Description:
In driver module ks7010, "checkpatch.pl" flags error for adding parenthesis 
around macro params.
Also, removing extra line.

Signed-off-by: Pushkar Jambhlekar 
---
 drivers/staging/ks7010/ks7010_sdio.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index a604c83..644b8d4 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -35,18 +35,18 @@ MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
 /* macro */
 
 #define inc_txqhead(priv) \
-   (priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
+   ((priv)->tx_dev.qhead = ((priv)->tx_dev.qhead + 1) % 
TX_DEVICE_BUFF_SIZE)
 #define inc_txqtail(priv) \
-   (priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
+   ((priv)->tx_dev.qtail = ((priv)->tx_dev.qtail + 1) % 
TX_DEVICE_BUFF_SIZE)
 #define cnt_txqbody(priv) \
-   (((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % 
TX_DEVICE_BUFF_SIZE)
+   priv)->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - 
((priv)->tx_dev.qhead)) % TX_DEVICE_BUFF_SIZE)
 
 #define inc_rxqhead(priv) \
-   (priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
+   ((priv)->rx_dev.qhead = ((priv)->rx_dev.qhead + 1) % 
RX_DEVICE_BUFF_SIZE)
 #define inc_rxqtail(priv) \
-   (priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
+   ((priv)->rx_dev.qtail = ((priv)->rx_dev.qtail + 1) % 
RX_DEVICE_BUFF_SIZE)
 #define cnt_rxqbody(priv) \
-   (((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % 
RX_DEVICE_BUFF_SIZE)
+   priv)->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - 
((priv)->rx_dev.qhead)) % RX_DEVICE_BUFF_SIZE)
 
 static int ks7010_sdio_read(struct ks_wlan_private *priv, unsigned int address,
unsigned char *buffer, int length)
@@ -884,7 +884,6 @@ static void ks7010_card_init(struct ks_wlan_private *priv)
if (priv->mac_address_valid && priv->version_size)
priv->dev_state = DEVICE_STATE_PREINIT;
 
-
hostif_sme_enqueue(priv, SME_GET_EEPROM_CKSUM);
 
/* load initial wireless parameter */
-- 
2.7.4

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


[PATCH] staging: media: atomisp: fix semicolon.cocci warnings

2017-03-13 Thread Julia Lawall
 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Signed-off-by: Julia Lawall 
Signed-off-by: Fengguang Wu 
---

The actual report raises a bunch of issues, but I didn't receive any more
detail about them:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
staging-testing
head:   940f6daec092819674ce26f1b6f0be4bbb9923a8
commit: 49637a458b61629672a8ae19fdae2058c64815cf [393/487] staging: media:
atomisp: remove '.' from pci Makefile
:: branch date: 2 hours ago
:: commit date: 2 days ago

>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5554:1-3:
WARNING: possible condition with no effect (if == else)
--
>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:4428:2-4:
ERROR: test of a variable/field address
--
>> drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:1774:2-28:
code aligned with following code on line 1776

(This one means that an if may be missing braces)

--
>>
drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:551:3-4:
Unneeded semicolon
--
>>
drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c:200:2-3:
Unneeded semicolon



 atomisp_compat_css20.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -548,7 +548,7 @@ static int __destroy_stream(struct atomi
}

usleep_range(100, 200);
-   };
+   }
}

stream_env->stream_state = CSS_STREAM_STOPPED;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 0/2] staging: speakup: checkpatch guided cleanups.

2017-03-13 Thread Arushi Singhal
Improve readability by fixing multiple checkpatch.pl
issues in speakup driver.

Arushi Singhal (2):
  staging: speakup: Add blank line after declarations
  staging: speakup: fix "Alignment match open parenthesis"

 drivers/staging/speakup/kobjects.c | 2 +-
 drivers/staging/speakup/main.c | 1 +
 drivers/staging/speakup/serialio.c | 1 +
 drivers/staging/speakup/speakup_dtlk.c | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.11.0

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


[PATCH v3 2/2] staging: speakup: fix "Alignment match open parenthesis"

2017-03-13 Thread Arushi Singhal
Alig arguments with open parenthesis.

Signed-off-by: Arushi Singhal 
---
 changes in v3
 - Improve the commit message.

 drivers/staging/speakup/kobjects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c 
b/drivers/staging/speakup/kobjects.c
index 8a586323b728..ca85476e3ff7 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -93,7 +93,7 @@ static void report_char_chartab_status(int reset, int 
received, int used,
} else if (received) {
len = snprintf(buf, sizeof(buf),
   " updated %d of %d %s\n",
-   used, received, object_type[do_characters]);
+  used, received, object_type[do_characters]);
if (rejected)
snprintf(buf + (len - 1), sizeof(buf) - (len - 1),
 " with %d reject%s\n",
-- 
2.11.0

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


[PATCH v3 1/2] staging: speakup: Add blank line after declarations

2017-03-13 Thread Arushi Singhal
Patch fixes the warnings reported by checkpatch.pl
for please use a blank line after function/struct/union/enum
declarations.
Add a blank line after enum and struct declarations.

Signed-off-by: Arushi Singhal 
---
 changes in v3
 - change the subject and commit message to make it more relevant.

 drivers/staging/speakup/main.c | 1 +
 drivers/staging/speakup/serialio.c | 1 +
 drivers/staging/speakup/speakup_dtlk.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 6786cfab7460..4b750ec8cd59 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -108,6 +108,7 @@ enum {
CT_Window,
CT_Max
 };
+
 #define read_all_mode CT_Max
 
 static struct tty_struct *tty;
diff --git a/drivers/staging/speakup/serialio.c 
b/drivers/staging/speakup/serialio.c
index aade52ee15a0..d51e382cb835 100644
--- a/drivers/staging/speakup/serialio.c
+++ b/drivers/staging/speakup/serialio.c
@@ -21,6 +21,7 @@ static void start_serial_interrupt(int irq);
 static const struct old_serial_port rs_table[] = {
SERIAL_PORT_DFNS
 };
+
 static const struct old_serial_port *serstate;
 static int timeouts;
 
diff --git a/drivers/staging/speakup/speakup_dtlk.c 
b/drivers/staging/speakup/speakup_dtlk.c
index eede2d82183a..c0b2208aa8a7 100644
--- a/drivers/staging/speakup/speakup_dtlk.c
+++ b/drivers/staging/speakup/speakup_dtlk.c
@@ -43,6 +43,7 @@ static int port_forced;
 static unsigned int synth_portlist[] = {
 0x25e, 0x29e, 0x2de, 0x31e, 0x35e, 0x39e, 0
 };
+
 static u_char synth_status;
 
 static struct var_t vars[] = {
-- 
2.11.0

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


[PATCH] Fixed a minor coding style warning. Arguments in the macros should be coverd in brackets to aviod any precedence issues.

2017-03-13 Thread mshan
Signed-off-by: mshan 
---
 drivers/staging/fwserial/fwserial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fwserial/fwserial.c 
b/drivers/staging/fwserial/fwserial.c
index 41a49c8..d693c03 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -98,7 +98,7 @@ struct fwtty_transaction {
};
 };
 
-#define to_device(a, b)(a->b)
+#define to_device(a, b)((a)->(b))
 #define fwtty_err(p, fmt, ...) \
dev_err(to_device(p, device), fmt, ##__VA_ARGS__)
 #define fwtty_info(p, fmt, ...)
\
-- 
2.7.4

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


Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline

2017-03-13 Thread Mauro Carvalho Chehab
Hi Sakari,

I started preparing a long argument about it, but gave up in favor of a
simpler one.

Em Mon, 13 Mar 2017 14:46:22 +0200
Sakari Ailus  escreveu:

> Drivers are written to support hardware, not particular use case.  

No, it is just the reverse: drivers and hardware are developed to
support use cases.

Btw, you should remember that the hardware is the full board, not just the
SoC. In practice, the board do limit the use cases: several provide a
single physical CSI connector, allowing just one sensor.

> > This situation is there since 2009. If I remember well, you tried to write
> > such generic plugin in the past, but never finished it, apparently because
> > it is too complex. Others tried too over the years.   
> 
> I'd argue I know better what happened with that attempt than you do. I had a
> prototype of a generic pipeline configuration library but due to various
> reasons I haven't been able to continue working on that since around 2012.

...

> > The last trial was done by Jacek, trying to cover just the exynos4 driver. 
> > Yet, even such limited scope plugin was not good enough, as it was never
> > merged upstream. Currently, there's no such plugins upstream.
> > 
> > If we can't even merge a plugin that solves it for just *one* driver,
> > I have no hope that we'll be able to do it for the generic case.  
> 
> I believe Jacek ceased to work on that plugin in his day job; other than
> that, there are some matters left to be addressed in his latest patchset.

The two above basically summaries the issue: the task of doing a generic
plugin on userspace, even for a single driver is complex enough to
not cover within a reasonable timeline.

>From 2009 to 2012, you were working on it, but didn't finish it.

Apparently, nobody worked on it between 2013-2014 (but I may be wrong, as
I didn't check when the generic plugin interface was added to libv4l).

In the case of Jacek's work, the first patch I was able to find was
written in Oct, 2014:
https://patchwork.kernel.org/patch/5098111/
(not sure what happened with the version 1).

The last e-mail about this subject was issued in Dec, 2016.

In summary, you had this on your task for 3 years for an OMAP3
plugin (where you have a good expertise), and Jacek for 2 years, 
for Exynos 4, where he should also have a good knowledge.

Yet, with all that efforts, no concrete results were achieved, as none
of the plugins got merged.

Even if they were merged, if we keep the same mean time to develop a
libv4l plugin, that would mean that a plugin for i.MX6 could take 2-3
years to be developed.

There's a clear message on it:
- we shouldn't keep pushing for a solution via libv4l.

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


Re: [PATCH] scsi: storvsc: Add support for FC rport.

2017-03-13 Thread Martin K. Petersen
> "Cathy" == Cathy Avery  writes:

Hi Cathy,

Cathy> I haven't received any feedback yet.

Cathy> Should I resend?

You sent this right at the beginning of the merge window. That almost
guarantees that nobody will have time to look at it.

Whereas now is a good time to send submissions for 4.12...

-- 
Martin K. Petersen  Oracle Linux Engineering
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2 V2] staging: dgnc: remove useless switch-case statements

2017-03-13 Thread Daeseok Youn
The dgnc_tty_send_break() has a switch-case condition for msec.
It is no use except case -1.

Signed-off-by: Daeseok Youn 
---
V2: The two patches in previous series are merged into one patch.

 drivers/staging/dgnc/dgnc_tty.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index dc76e9f..854bd1d 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1790,16 +1790,8 @@ static int dgnc_tty_send_break(struct tty_struct *tty, 
int msec)
if (!bd || bd->magic != DGNC_BOARD_MAGIC)
return -EIO;
 
-   switch (msec) {
-   case -1:
+   if (msec < 0)
msec = 0x;
-   break;
-   case 0:
-   msec = 0;
-   break;
-   default:
-   break;
-   }
 
spin_lock_irqsave(>ch_lock, flags);
 
-- 
1.9.1

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


[PATCH 1/2 V2] staging: dgnc: ch->ch_bd is already assigned to bd variable

2017-03-13 Thread Daeseok Youn
The bd variables in functions are already assigned from
ch->ch_bd but it is not used in those functions except checking NULL.

The ch->ch_bd could be replaced with bd variable.

Signed-off-by: Daeseok Youn 
---
V2: Patches in previous series are splited but it could be merged into one.
There are lines to replace ch->ch_bd with bd variable.

 drivers/staging/dgnc/dgnc_tty.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 1861bd5..dc76e9f 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1756,7 +1756,7 @@ static int dgnc_tty_tiocmset(struct tty_struct *tty,
if (clear & TIOCM_DTR)
ch->ch_mostat &= ~(UART_MCR_DTR);
 
-   ch->ch_bd->bd_ops->assert_modem_signals(ch);
+   bd->bd_ops->assert_modem_signals(ch);
 
spin_unlock_irqrestore(>ch_lock, flags);
 
@@ -1803,7 +1803,7 @@ static int dgnc_tty_send_break(struct tty_struct *tty, 
int msec)
 
spin_lock_irqsave(>ch_lock, flags);
 
-   ch->ch_bd->bd_ops->send_break(ch, msec);
+   bd->bd_ops->send_break(ch, msec);
 
spin_unlock_irqrestore(>ch_lock, flags);
 
@@ -2095,7 +2095,7 @@ static int dgnc_tty_digiseta(struct tty_struct *tty,
if (ch->ch_digi.digi_offlen > DIGI_PLEN)
ch->ch_digi.digi_offlen = DIGI_PLEN;
 
-   ch->ch_bd->bd_ops->param(tty);
+   bd->bd_ops->param(tty);
 
spin_unlock_irqrestore(>ch_lock, flags);
 
@@ -2136,7 +2136,7 @@ static void dgnc_tty_set_termios(struct tty_struct *tty,
ch->ch_startc = tty->termios.c_cc[VSTART];
ch->ch_stopc  = tty->termios.c_cc[VSTOP];
 
-   ch->ch_bd->bd_ops->param(tty);
+   bd->bd_ops->param(tty);
dgnc_carrier(ch);
 
spin_unlock_irqrestore(>ch_lock, flags);
-- 
1.9.1

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


Re: [PATCH 3/3] staging: dngc: ch->ch_bd is already assigned to bd variable

2017-03-13 Thread DaeSeok Youn
2017-03-14 7:26 GMT+09:00 Greg KH :
> On Sun, Mar 12, 2017 at 11:47:28PM +0900, Daeseok Youn wrote:
>> The bd variable in dgnc_tty_digiseta() is assigned with
>> ch->ch_bd but it is not used in this function except checking NULL.
>> The ch->ch_bd could be replaced with bd variable in dgnc_tty_digiseta()
>>
>> Signed-off-by: Daeseok Youn 
>> ---
>>  drivers/staging/dgnc/dgnc_tty.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Please merge this with your first patch in the series and resend them.
Hi Greg,

Ok. I will merge this with my first patch and send them.

Thanks,
Regards,
Daeseok Youn.

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


Re: [PATCH 03/12] staging: ks7010: convert comments to kernel doc format

2017-03-13 Thread Greg Kroah-Hartman
On Tue, Mar 14, 2017 at 12:39:46PM +1100, Tobin C. Harding wrote:
> On Tue, Mar 14, 2017 at 08:06:18AM +0800, Greg Kroah-Hartman wrote:
> > On Tue, Mar 14, 2017 at 09:54:01AM +1100, Tobin C. Harding wrote:
> > > Function comments use a custom format. We have a standard function
> > > comment format, kernel doc format. Using the standard format aids
> > > readability and allows documentation to be produced using kernel
> > > tools.
> > > 
> > > Convert function comments to use kernel doc format.
> > > 
> > > Signed-off-by: Tobin C. Harding 
> > > ---
> > >  drivers/staging/ks7010/ks_wlan_net.c | 554 
> > > +--
> > >  1 file changed, 395 insertions(+), 159 deletions(-)
> > > 
> > > diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
> > > b/drivers/staging/ks7010/ks_wlan_net.c
> > > index 1ff1948..c6f891e 100644
> > > --- a/drivers/staging/ks7010/ks_wlan_net.c
> > > +++ b/drivers/staging/ks7010/ks_wlan_net.c
> > > @@ -163,8 +163,7 @@ int ks_wlan_setup_parameter(struct ks_wlan_private 
> > > *priv,
> > >   return 0;
> > >  }
> > >  
> > > -/*
> > > - * Initial Wireless Extension code for Ks_Wlannet driver by :
> > > +/* Initial Wireless Extension code for Ks_Wlannet driver by :
> > >   *   Jean Tourrilhes  - HPL - 17 November 00
> > >   * Conversion to new driver API by :
> > >   *   Jean Tourrilhes  - HPL - 26 March 02
> > > @@ -173,8 +172,11 @@ int ks_wlan_setup_parameter(struct ks_wlan_private 
> > > *priv,
> > >   * would not work at all... - Jean II
> > >   */
> > >  
> > > -/*--*/
> > > -/* Wireless Handler : get protocol name */
> > > +/**
> > > + * ks_wlan_get_name() - Get protocol name.
> > > + *
> > > + * Wireless Handler
> > > + */
> > >  static int ks_wlan_get_name(struct net_device *dev,
> > 
> > static functions never need to be converted to kerneldoc, as no
> > documentation would need to be created for them.
> > 
> > All that needs to be done here is, at the most:
> > /* get protocol name */
> > 
> > But really, given that the function name is pretty good, there's no need
> > for a comment about it at all, right?
> > 
> > Same for lots of other ones in this patch.
> 
> Righto. What about the 'Wireless Handler' and 'Private Handler'. If we
> remove the comment won't that be some loss of information?

Do they actually make any sense?  Not to me...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 09/12] staging: ks7010: remove unnecessary cast

2017-03-13 Thread Tobin C. Harding
On Tue, Mar 14, 2017 at 08:07:15AM +0800, Greg Kroah-Hartman wrote:
> On Tue, Mar 14, 2017 at 09:54:07AM +1100, Tobin C. Harding wrote:
> > Return value from kmalloc() does not require a cast.
> > 
> > Remove unnecessary cast.
> > 
> > Signed-off-by: Tobin C. Harding 
> > ---
> >  drivers/staging/ks7010/ks_wlan_net.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
> > b/drivers/staging/ks7010/ks_wlan_net.c
> > index 1451ba7..e1e4a31 100644
> > --- a/drivers/staging/ks7010/ks_wlan_net.c
> > +++ b/drivers/staging/ks7010/ks_wlan_net.c
> > @@ -2561,7 +2561,7 @@ static int ks_wlan_data_write(struct net_device *dev,
> > if (priv->sleep_mode == SLP_SLEEP)
> > return -EPERM;
> > /* for SLEEP MODE */
> > -   wbuff = (unsigned char *)kmalloc(dwrq->length, GFP_ATOMIC);
> > +   wbuff = kmalloc(dwrq->length, GFP_ATOMIC);
> > if (!wbuff)
> > return -EFAULT;
> 
> This should also return -ENOMEM, but that's a separate fix :)

Good catch. I'll make a note of this and when/if the patch set gets
merged do a separate patch with this fix since it is not checkpatch
related.

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


Re: [PATCH 03/12] staging: ks7010: convert comments to kernel doc format

2017-03-13 Thread Tobin C. Harding
On Tue, Mar 14, 2017 at 08:06:18AM +0800, Greg Kroah-Hartman wrote:
> On Tue, Mar 14, 2017 at 09:54:01AM +1100, Tobin C. Harding wrote:
> > Function comments use a custom format. We have a standard function
> > comment format, kernel doc format. Using the standard format aids
> > readability and allows documentation to be produced using kernel
> > tools.
> > 
> > Convert function comments to use kernel doc format.
> > 
> > Signed-off-by: Tobin C. Harding 
> > ---
> >  drivers/staging/ks7010/ks_wlan_net.c | 554 
> > +--
> >  1 file changed, 395 insertions(+), 159 deletions(-)
> > 
> > diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
> > b/drivers/staging/ks7010/ks_wlan_net.c
> > index 1ff1948..c6f891e 100644
> > --- a/drivers/staging/ks7010/ks_wlan_net.c
> > +++ b/drivers/staging/ks7010/ks_wlan_net.c
> > @@ -163,8 +163,7 @@ int ks_wlan_setup_parameter(struct ks_wlan_private 
> > *priv,
> > return 0;
> >  }
> >  
> > -/*
> > - * Initial Wireless Extension code for Ks_Wlannet driver by :
> > +/* Initial Wireless Extension code for Ks_Wlannet driver by :
> >   * Jean Tourrilhes  - HPL - 17 November 00
> >   * Conversion to new driver API by :
> >   * Jean Tourrilhes  - HPL - 26 March 02
> > @@ -173,8 +172,11 @@ int ks_wlan_setup_parameter(struct ks_wlan_private 
> > *priv,
> >   * would not work at all... - Jean II
> >   */
> >  
> > -/*--*/
> > -/* Wireless Handler : get protocol name */
> > +/**
> > + * ks_wlan_get_name() - Get protocol name.
> > + *
> > + * Wireless Handler
> > + */
> >  static int ks_wlan_get_name(struct net_device *dev,
> 
> static functions never need to be converted to kerneldoc, as no
> documentation would need to be created for them.
> 
> All that needs to be done here is, at the most:
> /* get protocol name */
> 
> But really, given that the function name is pretty good, there's no need
> for a comment about it at all, right?
> 
> Same for lots of other ones in this patch.

Righto. What about the 'Wireless Handler' and 'Private Handler'. If we
remove the comment won't that be some loss of information?

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


Re: [RFC PATCH v2 06/32] x86/pci: Use memremap when walking setup data

2017-03-13 Thread Tom Lendacky

On 3/6/2017 6:03 PM, Bjorn Helgaas wrote:

On Fri, Mar 03, 2017 at 03:15:34PM -0600, Tom Lendacky wrote:

On 3/3/2017 2:42 PM, Bjorn Helgaas wrote:

On Thu, Mar 02, 2017 at 10:13:10AM -0500, Brijesh Singh wrote:

From: Tom Lendacky 

The use of ioremap will force the setup data to be mapped decrypted even
though setup data is encrypted.  Switch to using memremap which will be
able to perform the proper mapping.


How should callers decide whether to use ioremap() or memremap()?

memremap() existed before SME and SEV, and this code is used even if
SME and SEV aren't supported, so the rationale for this change should
not need the decryption argument.


When SME or SEV is active an ioremap() will remove the encryption bit
from the pagetable entry when it is mapped.  This allows MMIO, which
doesn't support SME/SEV, to be performed successfully.  So my take is
that ioremap() should be used for MMIO and memremap() for pages in RAM.


OK, thanks.  The commit message should say something like "this is
RAM, not MMIO, so we should map it with memremap(), not ioremap()".
That's the part that determines whether the change is correct.

You can mention the encryption part, too, but it's definitely
secondary because the change has to make sense on its own, without
SME/SEV.



Ok, that makes sense, will do.


The following commits (from https://github.com/codomania/tip/branches)
all do basically the same thing so the changelogs (and summaries)
should all be basically the same:

  cb0d0d1eb0a6 x86: Change early_ioremap to early_memremap for BOOT data
  91acb68b8333 x86/pci: Use memremap when walking setup data
  4f687503e23f x86: Access the setup data through sysfs decrypted
  e90246b8c229 x86: Access the setup data through debugfs decrypted

I would collect them all together and move them to the beginning of
your series, since they don't depend on anything else.


I'll do that.



Also, change "x86/pci: " to "x86/PCI" so it matches the previous
convention.


Will do.

Thanks,
Tom




Signed-off-by: Tom Lendacky 


Acked-by: Bjorn Helgaas 


---
arch/x86/pci/common.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index a4fdfa7..0b06670 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -691,7 +691,7 @@ int pcibios_add_device(struct pci_dev *dev)

pa_data = boot_params.hdr.setup_data;
while (pa_data) {
-   data = ioremap(pa_data, sizeof(*rom));
+   data = memremap(pa_data, sizeof(*rom), MEMREMAP_WB);


I can't quite connect the dots here.  ioremap() on x86 would do
ioremap_nocache().  memremap(MEMREMAP_WB) would do arch_memremap_wb(),
which is ioremap_cache().  Is making a cacheable mapping the important
difference?


The memremap(MEMREMAP_WB) will actually check to see if it can perform
a __va(pa_data) in try_ram_remap() and then fallback to the
arch_memremap_wb().  So it's actually the __va() vs the ioremap_cache()
that is the difference.

Thanks,
Tom




if (!data)
return -ENOMEM;

@@ -710,7 +710,7 @@ int pcibios_add_device(struct pci_dev *dev)
}
}
pa_data = data->next;
-   iounmap(data);
+   memunmap(data);
}
set_dma_domain_ops(dev);
set_dev_domain_options(dev);


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


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Samuel Thibault
Samuel Thibault, on mar. 14 mars 2017 01:47:01 +0100, wrote:
> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > On Mon, Mar 13, 2017 at 10:05:51PM +, okash.khaw...@gmail.com wrote:
> > > This patchset introduces a TTY-based way for the synths to communicate
> > > with devices as an alternate for direct serial comms used by the synths
> > > at the moment. It then migrates some of the synths to the TTY-based
> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > txprt.
> > 
> > What about using the serbus code that is now in the tree?  That should
> > make this a lot easier than your patchset from what I can see.
> 
> Mmm... AIUI from reading tty_port_register_device_attr, one
> would have to have registered a speakup serdev device driver
> *before* tty_port_register_device_attr gets called, so that
> serdev_tty_port_register matches the driver in the loop of
> of_serdev_register_devices, and no TTY cdev is created?
> 
> That would mean that speakup can not be loaded as a module after ttyS0
> initialization, that won't fly for our use needs. The line discipline
> mechanism allows us to attach ourself to an existing tty.  Could we
> imagine a tty_port function which removes the cdev and tries to register
> the tty port again to serdev?
> 
> What we basically need to be able to say on speakup module load is
> e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
> discuss with it".

That for_each_available_child_of_node loop is really way more complex
than what we need.  And what's more, it's not working without CONFIG_OF
(!)

It would really make sense to me to have a

serdev_device *tty_port_register_serdev_device(tty, device)

which unregisters the character device of the tty, and creates instead a
controler with the given device plugged to it. Really much like a line
discipline, but way simpler :)

The issue that remains is the "tty" part: we'd need there the tty_port,
the parent device, the tty driver, the idx. All we know when we load
speakup is "ttyS0". That's where going through the cdev to attach a line
discipline was simpler :)

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


Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

2017-03-13 Thread DaeSeok Youn
2017-03-14 2:54 GMT+09:00 Alan Cox :
>
> On Mon, 2017-03-13 at 19:54 +0900, Daeseok Youn wrote:
> > If the atomisp_kernel_zalloc() has "true" as a second parameter, it
> > tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
> > But using kzalloc is rather than kmalloc followed by memset with 0.
> > (vzalloc is for same reason with kzalloc)
>
> This is true but please don't apply this. There are about five other
> layers of indirection for memory allocators that want removing first so
> that the driver just uses the correct kmalloc/kzalloc/kv* functions in
> the right places.
right. kvmalloc/kvzalloc would be used after preparing those
interfaces in staging tree.
I will try to change all the atomisp_kernel_m{z}alloc() callers to
correct functions to allocate memory.

Thanks.
Regards,
Jake.

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


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Samuel Thibault
Hello,

Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> On Mon, Mar 13, 2017 at 10:05:51PM +, okash.khaw...@gmail.com wrote:
> > This patchset introduces a TTY-based way for the synths to communicate
> > with devices as an alternate for direct serial comms used by the synths
> > at the moment. It then migrates some of the synths to the TTY-based
> > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > txprt.
> 
> What about using the serbus code that is now in the tree?  That should
> make this a lot easier than your patchset from what I can see.

Mmm... AIUI from reading tty_port_register_device_attr, one
would have to have registered a speakup serdev device driver
*before* tty_port_register_device_attr gets called, so that
serdev_tty_port_register matches the driver in the loop of
of_serdev_register_devices, and no TTY cdev is created?

That would mean that speakup can not be loaded as a module after ttyS0
initialization, that won't fly for our use needs. The line discipline
mechanism allows us to attach ourself to an existing tty.  Could we
imagine a tty_port function which removes the cdev and tries to register
the tty port again to serdev?

What we basically need to be able to say on speakup module load is
e.g. "I'm now attaching a device to ttyS0, use this serdev_device_ops to
discuss with it".

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


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Samuel Thibault
Hello Okash,

I'd say for a start already, rebase and submit patches 2-5, since they
are refactoring that makes sense anyway. And we can discuss about serdev
while that gets integrated, and thus serdev-equivalents of patches 1 and
6 will then be immediatetly applicable.

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


Re: [PATCH 09/12] staging: ks7010: remove unnecessary cast

2017-03-13 Thread Greg Kroah-Hartman
On Tue, Mar 14, 2017 at 09:54:07AM +1100, Tobin C. Harding wrote:
> Return value from kmalloc() does not require a cast.
> 
> Remove unnecessary cast.
> 
> Signed-off-by: Tobin C. Harding 
> ---
>  drivers/staging/ks7010/ks_wlan_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
> b/drivers/staging/ks7010/ks_wlan_net.c
> index 1451ba7..e1e4a31 100644
> --- a/drivers/staging/ks7010/ks_wlan_net.c
> +++ b/drivers/staging/ks7010/ks_wlan_net.c
> @@ -2561,7 +2561,7 @@ static int ks_wlan_data_write(struct net_device *dev,
>   if (priv->sleep_mode == SLP_SLEEP)
>   return -EPERM;
>   /* for SLEEP MODE */
> - wbuff = (unsigned char *)kmalloc(dwrq->length, GFP_ATOMIC);
> + wbuff = kmalloc(dwrq->length, GFP_ATOMIC);
>   if (!wbuff)
>   return -EFAULT;

This should also return -ENOMEM, but that's a separate fix :)

thanks,

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


Re: [PATCH 03/12] staging: ks7010: convert comments to kernel doc format

2017-03-13 Thread Greg Kroah-Hartman
On Tue, Mar 14, 2017 at 09:54:01AM +1100, Tobin C. Harding wrote:
> Function comments use a custom format. We have a standard function
> comment format, kernel doc format. Using the standard format aids
> readability and allows documentation to be produced using kernel
> tools.
> 
> Convert function comments to use kernel doc format.
> 
> Signed-off-by: Tobin C. Harding 
> ---
>  drivers/staging/ks7010/ks_wlan_net.c | 554 
> +--
>  1 file changed, 395 insertions(+), 159 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
> b/drivers/staging/ks7010/ks_wlan_net.c
> index 1ff1948..c6f891e 100644
> --- a/drivers/staging/ks7010/ks_wlan_net.c
> +++ b/drivers/staging/ks7010/ks_wlan_net.c
> @@ -163,8 +163,7 @@ int ks_wlan_setup_parameter(struct ks_wlan_private *priv,
>   return 0;
>  }
>  
> -/*
> - * Initial Wireless Extension code for Ks_Wlannet driver by :
> +/* Initial Wireless Extension code for Ks_Wlannet driver by :
>   *   Jean Tourrilhes  - HPL - 17 November 00
>   * Conversion to new driver API by :
>   *   Jean Tourrilhes  - HPL - 26 March 02
> @@ -173,8 +172,11 @@ int ks_wlan_setup_parameter(struct ks_wlan_private *priv,
>   * would not work at all... - Jean II
>   */
>  
> -/*--*/
> -/* Wireless Handler : get protocol name */
> +/**
> + * ks_wlan_get_name() - Get protocol name.
> + *
> + * Wireless Handler
> + */
>  static int ks_wlan_get_name(struct net_device *dev,

static functions never need to be converted to kerneldoc, as no
documentation would need to be created for them.

All that needs to be done here is, at the most:
/* get protocol name */

But really, given that the function name is pretty good, there's no need
for a comment about it at all, right?

Same for lots of other ones in this patch.

thanks,

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


Re: [PATCH] staging: rtl8188eu: removed blank lines coding style problem

2017-03-13 Thread Greg KH
On Mon, Mar 13, 2017 at 09:00:49AM +0200, Andrii wrote:
> Fix 'multiple blank lines' coding style problem reported by
> checkpatch.pl.
> 
> Signed-off-by: Andrii Vladyka 

Your From: name didn't match up with your signed-off-by: name here :(

Please fix up and resend.

thanks,

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


Re: [PATCH 1/2] staging: wlan-ng: introduce a macro read_u16

2017-03-13 Thread Greg KH
On Mon, Mar 13, 2017 at 02:14:25PM +0100, Gioh Kim wrote:
> read_u16 is wrapper of le16_to_cpu to read u16 variable,
> rather than __le16.
> 
> Signed-off-by: Gioh Kim 
> ---
>  drivers/staging/wlan-ng/prism2mgmt.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/wlan-ng/prism2mgmt.h 
> b/drivers/staging/wlan-ng/prism2mgmt.h
> index 88b979f..97ede25 100644
> --- a/drivers/staging/wlan-ng/prism2mgmt.h
> +++ b/drivers/staging/wlan-ng/prism2mgmt.h
> @@ -63,6 +63,9 @@
>  extern int prism2_reset_holdtime;
>  extern int prism2_reset_settletime;
>  
> +#define read_u16(x) ({ u32 __r = (u32)le16_to_cpu((__force __le16)(x)); \
> + __r; })
> +

Eeek, no, this should not be needed at all, if the code is written
correctly.  Please fix things up to work properly, don't paper over them
with forced casts.

thanks,

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


Re: [PATCH] staging: rtl8192u: use __le16_to_cpu instead of cast

2017-03-13 Thread Greg KH
On Sun, Mar 12, 2017 at 04:02:35PM -0600, Perry Hooker wrote:
> This patch fixes the following sparse warnings:
> drivers/staging/rtl8192u/r8192U_dm.c:2307:49: warning: cast from restricted 
> __le16
> drivers/staging/rtl8192u/r8192U_dm.c:2308:44: warning: cast from restricted 
> __le16
> drivers/staging/rtl8192u/r8192U_dm.c:2309:44: warning: cast from restricted 
> __le16
> 
> Signed-off-by: Perry Hooker 
> ---
>  drivers/staging/rtl8192u/r8192U_dm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

This patch does not apply to my tree at all :(

Please rebase it against linux-next and try again.

thanks,

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


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Samuel Thibault
Samuel Thibault, on lun. 13 mars 2017 23:26:08 +0100, wrote:
> > That should make this a lot easier than your patchset from what I can
> > see.
> 
> Will serdev support modem line control?  We need this for some devices.

"serdev: add serdev_device_set_rts" seems to be saying yes :)

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


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Samuel Thibault
Greg KH, on mar. 14 mars 2017 07:43:03 +0800, wrote:
> > Well, for a start that didn't exist when Okash worked on the TTY part :)
> 
> Fair enough, but it has been discussed for many months now on the
> linux-serial mailing list.

I have not had time for years to actually work on this subject, so I've
had even less time to follow linux-serial :) (and Okash, who actually
got the time to do the work, is very new to all this).

> > > That should make this a lot easier than your patchset from what I can
> > > see.
> > 
> > Will serdev support modem line control?  We need this for some devices.
> > How is the serial port chosen with serdev?
> 
> Best asked on the linux-serial mailing list :)

Let's discuss there, then.

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


[PATCH v2] staging: ks7010: fix spelling of Michael MIC

2017-03-13 Thread Tobin C. Harding
Driver mixes spelling michael and michel in symbol names and
comments. Michael here references the IEEE 802.11i Message Integrity
Code. It is incorrect to spell it michel and confusing having two
spellings for the same thing.

Change michel -> micheal in both symbol names and comments.

Signed-off-by: Tobin C. Harding 
---

v1 -> v2:
 - rebase onto branch: staging-testing

 drivers/staging/ks7010/ks_hostif.c   | 16 
 drivers/staging/ks7010/michael_mic.c |  8 
 drivers/staging/ks7010/michael_mic.h |  6 +++---
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index 310928f..1e49717 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -317,7 +317,7 @@ int hostif_data_indication_wpa(struct ks_wlan_private *priv,
char buf[128];
unsigned long now;
struct mic_failure_t *mic_failure;
-   struct michel_mic_t michel_mic;
+   struct michael_mic_t michael_mic;
union iwreq_data wrqu;
unsigned int key_index = auth_type - 1;
struct wpa_key_t *key = >wpa.key[key_index];
@@ -347,14 +347,14 @@ int hostif_data_indication_wpa(struct ks_wlan_private 
*priv,
memcpy([0], (priv->rxp) + ((priv->rx_size) - 8), 8);
priv->rx_size = priv->rx_size - 8;
if (auth_type > 0 && auth_type < 4) {   /* auth_type check */
-   MichaelMICFunction(_mic,
+   MichaelMICFunction(_mic,
   (uint8_t *)key->rx_mic_key,
   (uint8_t *)priv->rxp,
   (int)priv->rx_size,
   (uint8_t)0,  /* priority */
-  (uint8_t *)michel_mic.Result);
+  (uint8_t *)michael_mic.Result);
}
-   if (memcmp(michel_mic.Result, RecvMIC, 8)) {
+   if (memcmp(michael_mic.Result, RecvMIC, 8)) {
now = jiffies;
mic_failure = >wpa.mic_failure;
/* MIC FAILURE */
@@ -1120,7 +1120,7 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
int result = 0;
unsigned short eth_proto;
struct ether_hdr *eth_hdr;
-   struct michel_mic_t michel_mic;
+   struct michael_mic_t michael_mic;
unsigned short keyinfo = 0;
struct ieee802_1x_hdr *aa1x_hdr;
struct wpa_eapol_key *eap_key;
@@ -1228,10 +1228,10 @@ int hostif_data_request(struct ks_wlan_private *priv, 
struct sk_buff *packet)
pp->auth_type = cpu_to_le16((uint16_t)TYPE_AUTH);   
/* no encryption */
} else {
if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) {
-   MichaelMICFunction(_mic, (uint8_t 
*)priv->wpa.key[0].tx_mic_key, (uint8_t *)>data[0], (int)packet_len, 
(uint8_t)0,   /* priority */
-  (uint8_t *)michel_mic.
+   MichaelMICFunction(_mic, (uint8_t 
*)priv->wpa.key[0].tx_mic_key, (uint8_t *)>data[0], (int)packet_len, 
(uint8_t)0,  /* priority */
+  (uint8_t *)michael_mic.
   Result);
-   memcpy(p, michel_mic.Result, 8);
+   memcpy(p, michael_mic.Result, 8);
length += 8;
packet_len += 8;
p += 8;
diff --git a/drivers/staging/ks7010/michael_mic.c 
b/drivers/staging/ks7010/michael_mic.c
index f6e70fa..de5e724 100644
--- a/drivers/staging/ks7010/michael_mic.c
+++ b/drivers/staging/ks7010/michael_mic.c
@@ -38,7 +38,7 @@ do {  \
 } while (0)
 
 static
-void MichaelInitializeFunction(struct michel_mic_t *Mic, uint8_t *key)
+void MichaelInitializeFunction(struct michael_mic_t *Mic, uint8_t *key)
 {
// Set the key
Mic->K0 = getUInt32(key, 0);
@@ -61,7 +61,7 @@ do {  
\
 } while (0)
 
 static
-void MichaelAppend(struct michel_mic_t *Mic, uint8_t *src, int nBytes)
+void MichaelAppend(struct michael_mic_t *Mic, uint8_t *src, int nBytes)
 {
int addlen;
 
@@ -96,7 +96,7 @@ void MichaelAppend(struct michel_mic_t *Mic, uint8_t *src, 
int nBytes)
 }
 
 static
-void MichaelGetMIC(struct michel_mic_t *Mic, uint8_t *dst)
+void MichaelGetMIC(struct michael_mic_t *Mic, uint8_t *dst)
 {
u8 *data = Mic->M;
 
@@ -125,7 +125,7 @@ void MichaelGetMIC(struct michel_mic_t *Mic, uint8_t *dst)
MichaelClear(Mic);
 }
 
-void MichaelMICFunction(struct michel_mic_t *Mic, u8 *Key,
+void 

Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Greg Kroah-Hartman
On Mon, Mar 13, 2017 at 11:26:08PM +0100, Samuel Thibault wrote:
> Hello,
> 
> Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> > On Mon, Mar 13, 2017 at 10:05:51PM +, okash.khaw...@gmail.com wrote:
> > > This patchset introduces a TTY-based way for the synths to communicate
> > > with devices as an alternate for direct serial comms used by the synths
> > > at the moment. It then migrates some of the synths to the TTY-based
> > > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > > txprt.
> > 
> > What about using the serbus code that is now in the tree?
> 
> I guess you mean serdev?

Sorry, yes.

> Well, for a start that didn't exist when Okash worked on the TTY part :)

Fair enough, but it has been discussed for many months now on the
linux-serial mailing list.

> > That should make this a lot easier than your patchset from what I can
> > see.
> 
> Will serdev support modem line control?  We need this for some devices.
> How is the serial port chosen with serdev?

Best asked on the linux-serial mailing list :)

thanks,

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


Re: [PATCH v5 00/39] i.MX Media Driver

2017-03-13 Thread Steve Longerbeam

er, I meant I will integrate this patch. And verify/fix
possible breakage for non-bayer passthrough.

Steve


On 03/13/2017 02:30 AM, Russell King - ARM Linux wrote:

On Mon, Mar 13, 2017 at 08:16:25AM +, Russell King - ARM Linux wrote:

On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:

On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:

What I had was this patch for your v3.  I never got to testing your
v4 because of the LP-11 problem.

In v5, you've changed to propagate the ipu_cpmem_set_image() error
code to avoid the resulting corruption, but that leaves the other bits
of this patch unaddressed, along my "media: imx: smfc: add support
for bayer formats" patch.

Your driver basically has no support for bayer formats.

You added the patches to this driver that adds the bayer support,
I don't think there is anything more required of the driver at this
point to support bayer, the remaining work needs to happen in the IPUv3
driver.

There is more work, because the way you've merged my changes to
imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
respect to the burst size.

You always set it to 8 or 16 depending on the width:

burst_size = (image.pix.width & 0xf) ? 8 : 16;

ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);

and then you have my switch() statement which assigns burst_size.
My _tested_ code removed the above, added the switch, which had
a default case which reflected the above setting:

default:
burst_size = (outfmt->width & 0xf) ? 8 : 16;

and then went on to set the burst size _after_ the switch statement:

ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);

The effect is unchanged for non-bayer formats.  For bayer formats, the
burst size is determined by the bayer data size.

So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
above is still required.

I'm not convinced that fixing ipu_cpmem_set_image() is even the best
solution - it's not as trivial as it looks on the surface:

 ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
 ipu_cpmem_set_stride(ch, pix->bytesperline);

this is fine, it doesn't depend on the format.  However, the next line:

 ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));

does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
DRM knows nothing about bayer formats, there aren't fourcc codes in
DRM for it.  The result is that v4l2_pix_fmt_to_drm_fourcc() returns
-EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().

ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
it's a bug that this is not checked and propagated.  If it is checked and
propagated, then we need this to support bayer formats, and I don't see
DRM people wanting bayer format fourcc codes added without there being
a real DRM driver wanting to use them.

Then there's the business of calculating the top-left offset of the image,
which for bayer always needs to be an even number of pixels - as this
function takes the top-left offset, it ought to respect it, but if it
doesn't meet this criteria, what should it do?  csi_idmac_setup_channel()
always sets them to zero, but that's not really something that
ipu_cpmem_set_image() should assume.

For the time being, I've restored the functionality along the same lines
as I originally had.  This seems to get me working capture, but might
break non-bayer passthrough mode:

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index fc0036aa84d0..df336971a009 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -314,14 +314,6 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
image.phys0 = phys[0];
image.phys1 = phys[1];
  
-	ret = ipu_cpmem_set_image(priv->idmac_ch, );

-   if (ret)
-   return ret;
-
-   burst_size = (image.pix.width & 0xf) ? 8 : 16;
-
-   ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
-
/*
 * Check for conditions that require the IPU to handle the
 * data internally as generic data, aka passthrough mode:
@@ -346,15 +338,29 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
passthrough_bits = 16;
break;
default:
+   burst_size = (image.pix.width & 0xf) ? 8 : 16;
passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
   sensor_ep->bus.parallel.bus_width >= 16);
passthrough_bits = 16;
break;
}
  
-	if (passthrough)

+   if (passthrough) {
+   ipu_cpmem_set_resolution(priv->idmac_ch, image.rect.width,
+image.rect.height);
+   ipu_cpmem_set_stride(priv->idmac_ch, 

Re: [PATCH v5 00/39] i.MX Media Driver

2017-03-13 Thread Steve Longerbeam



On 03/13/2017 01:16 AM, Russell King - ARM Linux wrote:

On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:

On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:

What I had was this patch for your v3.  I never got to testing your
v4 because of the LP-11 problem.

In v5, you've changed to propagate the ipu_cpmem_set_image() error
code to avoid the resulting corruption, but that leaves the other bits
of this patch unaddressed, along my "media: imx: smfc: add support
for bayer formats" patch.

Your driver basically has no support for bayer formats.

You added the patches to this driver that adds the bayer support,
I don't think there is anything more required of the driver at this
point to support bayer, the remaining work needs to happen in the IPUv3
driver.

There is more work, because the way you've merged my changes to
imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
respect to the burst size.

You always set it to 8 or 16 depending on the width:

burst_size = (image.pix.width & 0xf) ? 8 : 16;

ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);

and then you have my switch() statement which assigns burst_size.
My _tested_ code removed the above, added the switch, which had
a default case which reflected the above setting:

default:
burst_size = (outfmt->width & 0xf) ? 8 : 16;

and then went on to set the burst size _after_ the switch statement:

ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);

The effect is unchanged for non-bayer formats.  For bayer formats, the
burst size is determined by the bayer data size.

So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
above is still required.


Oops, sorry missed that. I'll fix.



I'm not convinced that fixing ipu_cpmem_set_image() is even the best
solution - it's not as trivial as it looks on the surface:

 ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
 ipu_cpmem_set_stride(ch, pix->bytesperline);

this is fine, it doesn't depend on the format.  However, the next line:

 ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));

does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
DRM knows nothing about bayer formats, there aren't fourcc codes in
DRM for it.


right, yeah that's a problem.


   The result is that v4l2_pix_fmt_to_drm_fourcc() returns
-EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().


Ugh.



ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
it's a bug that this is not checked and propagated.  If it is checked and
propagated, then we need this to support bayer formats, and I don't see
DRM people wanting bayer format fourcc codes added without there being
a real DRM driver wanting to use them.


true.



Then there's the business of calculating the top-left offset of the image,
which for bayer always needs to be an even number of pixels - as this
function takes the top-left offset, it ought to respect it, but if it
doesn't meet this criteria, what should it do?  csi_idmac_setup_channel()
always sets them to zero, but that's not really something that
ipu_cpmem_set_image() should assume.


Well, I will integrate your patch above. Thanks for doing this
work for me.

We do need to address the issues you brought up in ipu_cpmem at
some point.

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


[PATCH 07/12] staging: ks7010: move comparison to right hand side

2017-03-13 Thread Tobin C. Harding
Checkpatch emits WARNING: Comparisons should place the constant on the
right side of the test.

Move constant to right hand side of test, modify operator to ensure
logic is maintained.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index 59eabb9..cee3971 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2164,7 +2164,7 @@ static int ks_wlan_set_pmksa(struct net_device *dev,
}
}
if (ptr == >pmklist.head) {   /* not find 
address. */
-   if (PMK_LIST_MAX > priv->pmklist.size) {
/* new cache data */
+   if (priv->pmklist.size <= PMK_LIST_MAX) {   
/* new cache data */
for (i = 0; i < PMK_LIST_MAX; i++) {
pmk = >pmklist.pmk[i];
if (!memcmp
-- 
2.7.4

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


[PATCH 11/12] staging: ks7010: reduce level of indentation

2017-03-13 Thread Tobin C. Harding
Checkpatch emits WARNING: Too many leading tabs - consider code
refactoring. One level of indentation may be removed by inverting an
if statement conditional (and returning if new conditional evaluates
to true). Code contains switch statement that also contains multiple
layers of indentation. Indentation may be reduced by breaking out of
the case statement in multiple places instead of guarding code with
if/else statements.

Invert conditional. Return original error code if new conditional
evaluates to true. Break out of case blocks instead of using
if/else. Do not modify program logic.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 195 ++-
 1 file changed, 98 insertions(+), 97 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index bb20bb9..ca46869 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2004,72 +2004,71 @@ static int ks_wlan_set_encode_ext(struct net_device 
*dev,
if (dwrq->flags & IW_ENCODE_DISABLED)
priv->wpa.key[index].key_len = 0;
 
-   if (enc) {
-   priv->wpa.key[index].ext_flags = enc->ext_flags;
-   if (enc->ext_flags & IW_ENCODE_EXT_SET_TX_KEY) {
-   priv->wpa.txkey = index;
-   commit |= SME_WEP_INDEX;
-   } else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) {
-   memcpy(>wpa.key[index].rx_seq[0],
-  enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
-   }
+   if (!enc)
+   return -EINVAL;
 
-   memcpy(>wpa.key[index].addr.sa_data[0],
-  >addr.sa_data[0], ETH_ALEN);
+   priv->wpa.key[index].ext_flags = enc->ext_flags;
+   if (enc->ext_flags & IW_ENCODE_EXT_SET_TX_KEY) {
+   priv->wpa.txkey = index;
+   commit |= SME_WEP_INDEX;
+   } else if (enc->ext_flags & IW_ENCODE_EXT_RX_SEQ_VALID) {
+   memcpy(>wpa.key[index].rx_seq[0],
+   enc->rx_seq, IW_ENCODE_SEQ_MAX_SIZE);
+   }
 
-   switch (enc->alg) {
-   case IW_ENCODE_ALG_NONE:
-   if (priv->reg.privacy_invoked) {
-   priv->reg.privacy_invoked = 0x00;
-   commit |= SME_WEP_FLAG;
-   }
-   priv->wpa.key[index].key_len = 0;
+   memcpy(>wpa.key[index].addr.sa_data[0],
+   >addr.sa_data[0], ETH_ALEN);
 
-   break;
-   case IW_ENCODE_ALG_WEP:
-   case IW_ENCODE_ALG_CCMP:
-   if (!priv->reg.privacy_invoked) {
-   priv->reg.privacy_invoked = 0x01;
-   commit |= SME_WEP_FLAG;
-   }
-   if (enc->key_len) {
-   memcpy(>wpa.key[index].key_val[0],
-  >key[0], enc->key_len);
-   priv->wpa.key[index].key_len = enc->key_len;
-   commit |= (SME_WEP_VAL1 << index);
-   }
-   break;
-   case IW_ENCODE_ALG_TKIP:
-   if (!priv->reg.privacy_invoked) {
-   priv->reg.privacy_invoked = 0x01;
-   commit |= SME_WEP_FLAG;
-   }
-   if (enc->key_len == 32) {
-   memcpy(>wpa.key[index].key_val[0],
-  >key[0], enc->key_len - 16);
-   priv->wpa.key[index].key_len =
-   enc->key_len - 16;
-   if (priv->wpa.key_mgmt_suite == 4) {/* 
WPA_NONE */
-   memcpy(>wpa.key[index].
-  tx_mic_key[0], >key[16], 8);
-   memcpy(>wpa.key[index].
-  rx_mic_key[0], >key[16], 8);
-   } else {
-   memcpy(>wpa.key[index].
-  tx_mic_key[0], >key[16], 8);
-   memcpy(>wpa.key[index].
-  rx_mic_key[0], >key[24], 8);
-   }
-   commit |= (SME_WEP_VAL1 << index);
+   switch (enc->alg) {
+   case IW_ENCODE_ALG_NONE:
+   if (priv->reg.privacy_invoked) {
+   priv->reg.privacy_invoked = 0x00;
+   commit |= SME_WEP_FLAG;
+   }
+   priv->wpa.key[index].key_len = 0;
+
+   break;
+   case IW_ENCODE_ALG_WEP:
+   

[PATCH 12/12] staging: ks7010: refactor, whitespace only

2017-03-13 Thread Tobin C. Harding
Code may be refactored to take advantage of previous patches which
reduced the level of indentation. Function parameter line breaks can
be adjusted in line with kernel coding standards.

Refactor layout of function call parameters. Make whitespace changes
only.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 36 +---
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index ca46869..4d86d8a 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2156,10 +2156,8 @@ static int ks_wlan_set_pmksa(struct net_device *dev,
list_for_each(ptr, >pmklist.head) {
pmk = list_entry(ptr, struct pmk_t, list);
if (!memcmp(pmksa->bssid.sa_data, pmk->bssid, 
ETH_ALEN)) {  /* match address! list move to head. */
-   memcpy(pmk->pmkid, pmksa->pmkid,
-   IW_PMKID_LEN);
-   list_move(>list,
-   >pmklist.head);
+   memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
+   list_move(>list, >pmklist.head);
break; /* list_for_each */
}
}
@@ -2169,28 +2167,20 @@ static int ks_wlan_set_pmksa(struct net_device *dev,
if (priv->pmklist.size <= PMK_LIST_MAX) {   /* new cache 
data */
for (i = 0; i < PMK_LIST_MAX; i++) {
pmk = >pmklist.pmk[i];
-   if (!memcmp
-   ("\x00\x00\x00\x00\x00\x00",
-   pmk->bssid, ETH_ALEN))
-   break;
+   if (!memcmp("\x00\x00\x00\x00\x00\x00",
+   pmk->bssid, ETH_ALEN))
+   break; /* loop */
}
-   memcpy(pmk->bssid, pmksa->bssid.sa_data,
-   ETH_ALEN);
-   memcpy(pmk->pmkid, pmksa->pmkid,
-   IW_PMKID_LEN);
-   list_add(>list,
-   >pmklist.head);
+   memcpy(pmk->bssid, pmksa->bssid.sa_data, ETH_ALEN);
+   memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
+   list_add(>list, >pmklist.head);
priv->pmklist.size++;
} else {/* overwrite old cache data */
-   pmk =
-   list_entry(priv->pmklist.head.prev,
-   struct pmk_t, list);
-   memcpy(pmk->bssid, pmksa->bssid.sa_data,
-   ETH_ALEN);
-   memcpy(pmk->pmkid, pmksa->pmkid,
-   IW_PMKID_LEN);
-   list_move(>list,
-   >pmklist.head);
+   pmk = list_entry(priv->pmklist.head.prev, struct pmk_t,
+list);
+   memcpy(pmk->bssid, pmksa->bssid.sa_data, ETH_ALEN);
+   memcpy(pmk->pmkid, pmksa->pmkid, IW_PMKID_LEN);
+   list_move(>list, >pmklist.head);
}
break;
case IW_PMKSA_REMOVE:
-- 
2.7.4

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


[PATCH 03/12] staging: ks7010: convert comments to kernel doc format

2017-03-13 Thread Tobin C. Harding
Function comments use a custom format. We have a standard function
comment format, kernel doc format. Using the standard format aids
readability and allows documentation to be produced using kernel
tools.

Convert function comments to use kernel doc format.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 554 +--
 1 file changed, 395 insertions(+), 159 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index 1ff1948..c6f891e 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -163,8 +163,7 @@ int ks_wlan_setup_parameter(struct ks_wlan_private *priv,
return 0;
 }
 
-/*
- * Initial Wireless Extension code for Ks_Wlannet driver by :
+/* Initial Wireless Extension code for Ks_Wlannet driver by :
  * Jean Tourrilhes  - HPL - 17 November 00
  * Conversion to new driver API by :
  * Jean Tourrilhes  - HPL - 26 March 02
@@ -173,8 +172,11 @@ int ks_wlan_setup_parameter(struct ks_wlan_private *priv,
  * would not work at all... - Jean II
  */
 
-/*--*/
-/* Wireless Handler : get protocol name */
+/**
+ * ks_wlan_get_name() - Get protocol name.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_get_name(struct net_device *dev,
struct iw_request_info *info, char *cwrq,
char *extra)
@@ -198,8 +200,11 @@ static int ks_wlan_get_name(struct net_device *dev,
return 0;
 }
 
-/*--*/
-/* Wireless Handler : set frequency */
+/**
+ * ks_wlan_set_freq() - Set frequency.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_set_freq(struct net_device *dev,
struct iw_request_info *info, struct iw_freq *fwrq,
char *extra)
@@ -247,8 +252,11 @@ static int ks_wlan_set_freq(struct net_device *dev,
return rc;
 }
 
-/*--*/
-/* Wireless Handler : get frequency */
+/**
+ * ks_wlan_get_freq() - Get frequency.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_get_freq(struct net_device *dev,
struct iw_request_info *info, struct iw_freq *fwrq,
char *extra)
@@ -271,8 +279,11 @@ static int ks_wlan_get_freq(struct net_device *dev,
return 0;
 }
 
-/*--*/
-/* Wireless Handler : set ESSID */
+/**
+ * ks_wlan_set_essid() - Set ESSID.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_set_essid(struct net_device *dev,
 struct iw_request_info *info,
 struct iw_point *dwrq, char *extra)
@@ -330,8 +341,11 @@ static int ks_wlan_set_essid(struct net_device *dev,
return 0;
 }
 
-/*--*/
-/* Wireless Handler : get ESSID */
+/**
+ * ks_wlan_get_essid() - Get ESSID.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_get_essid(struct net_device *dev,
 struct iw_request_info *info,
 struct iw_point *dwrq, char *extra)
@@ -374,8 +388,11 @@ static int ks_wlan_get_essid(struct net_device *dev,
return 0;
 }
 
-/*--*/
-/* Wireless Handler : set AP address */
+/**
+ * ks_wlan_set_wap() - Set AP address.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_set_wap(struct net_device *dev, struct iw_request_info 
*info,
   struct sockaddr *ap_addr, char *extra)
 {
@@ -410,8 +427,11 @@ static int ks_wlan_set_wap(struct net_device *dev, struct 
iw_request_info *info,
return 0;
 }
 
-/*--*/
-/* Wireless Handler : get AP address */
+/**
+ * ks_wlan_get_wap() - Get AP address.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_get_wap(struct net_device *dev, struct iw_request_info 
*info,
   struct sockaddr *awrq, char *extra)
 {
@@ -432,8 +452,11 @@ static int ks_wlan_get_wap(struct net_device *dev, struct 
iw_request_info *info,
return 0;
 }
 
-/*--*/
-/* Wireless Handler : set Nickname */
+/**
+ * ks_wlan_set_nick() - Set Nickname.
+ *
+ * Wireless Handler
+ */
 static int ks_wlan_set_nick(struct net_device *dev,
struct iw_request_info *info, struct iw_point *dwrq,
char *extra)
@@ -455,8 +478,11 @@ static int ks_wlan_set_nick(struct net_device *dev,
return -EINPROGRESS;/* Call commit handler */
 }
 
-/*--*/
-/* Wireless Handler : get Nickname */
+/**
+ * 

[PATCH 04/12] staging: ks7010: fix logical line continuation

2017-03-13 Thread Tobin C. Harding
Checkpatch emits CHECK: Logical continuations should be on the
previous line.

Move logical continuation to previous line.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index c6f891e..bae4011 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1501,8 +1501,8 @@ static int ks_wlan_set_scan(struct net_device *dev,
 
/* for SLEEP MODE */
/* specified SSID SCAN */
-   if (wrqu->data.length == sizeof(struct iw_scan_req)
-   && wrqu->data.flags & IW_SCAN_THIS_ESSID) {
+   if (wrqu->data.length == sizeof(struct iw_scan_req) &&
+   wrqu->data.flags & IW_SCAN_THIS_ESSID) {
req = (struct iw_scan_req *)extra;
priv->scan_ssid_len = req->essid_len;
memcpy(priv->scan_ssid, req->essid, priv->scan_ssid_len);
-- 
2.7.4

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


[PATCH 08/12] staging: ks7010: remove unnecessary else statement

2017-03-13 Thread Tobin C. Harding
Checkpatch emits WARNING: else is not generally useful after a break
or return. Two warnings of this type are emitted, both are the result
of a else statement after a return statement. The 'else' can safely be
removed.

Remove unnecessary else statement.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index cee3971..1451ba7 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1999,8 +1999,7 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
/* for SLEEP MODE */
if (index < 1 || index > 4)
return -EINVAL;
-   else
-   index--;
+   index--;
 
if (dwrq->flags & IW_ENCODE_DISABLED)
priv->wpa.key[index].key_len = 0;
@@ -2196,20 +2195,21 @@ static int ks_wlan_set_pmksa(struct net_device *dev,
case IW_PMKSA_REMOVE:
if (list_empty(>pmklist.head)) {  /* list empty */
return -EINVAL;
-   } else {/* search cache data */
-   list_for_each(ptr, >pmklist.head) {
-   pmk = list_entry(ptr, struct pmk_t, list);
-   if (!memcmp(pmksa->bssid.sa_data, pmk->bssid, 
ETH_ALEN)) {  /* match address! list del. */
-   eth_zero_addr(pmk->bssid);
-   memset(pmk->pmkid, 0, IW_PMKID_LEN);
-   list_del_init(>list);
-   break;
-   }
-   }
-   if (ptr == >pmklist.head) {   /* not find 
address. */
-   return 0;
+   }
+   /* search cache data */
+   list_for_each(ptr, >pmklist.head) {
+   pmk = list_entry(ptr, struct pmk_t, list);
+   if (!memcmp(pmksa->bssid.sa_data, pmk->bssid, 
ETH_ALEN)) {  /* match address! list del. */
+   eth_zero_addr(pmk->bssid);
+   memset(pmk->pmkid, 0, IW_PMKID_LEN);
+   list_del_init(>list);
+   break;
}
}
+   if (ptr == >pmklist.head) {   /* not find address. */
+   return 0;
+   }
+
break;
case IW_PMKSA_FLUSH:
memset(>pmklist, 0, sizeof(priv->pmklist));
-- 
2.7.4

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


[PATCH 10/12] staging: ks7010: fix checkpatch memset warning

2017-03-13 Thread Tobin C. Harding
Checkpatch emits WARNING: single byte memset is suspicious. Swapped
2nd/3rd argument? Call site in question is correct but is an unusual
use of memset() to zero a single byte. The same can be achieved by
assigning 0 directly to the memory location.

Dereference pointer and assign 0 to that memory location. Use '\0' to
make explicit that it is a char pointer.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index e1e4a31..bb20bb9 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2595,7 +2595,7 @@ static int ks_wlan_data_read(struct net_device *dev,
return 0;
}
read_length = 0;
-   memset(extra, 0, 1);
+   *extra = '\0';
dwrq->length = 0;
return 0;
}
-- 
2.7.4

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


[PATCH 09/12] staging: ks7010: remove unnecessary cast

2017-03-13 Thread Tobin C. Harding
Return value from kmalloc() does not require a cast.

Remove unnecessary cast.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index 1451ba7..e1e4a31 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -2561,7 +2561,7 @@ static int ks_wlan_data_write(struct net_device *dev,
if (priv->sleep_mode == SLP_SLEEP)
return -EPERM;
/* for SLEEP MODE */
-   wbuff = (unsigned char *)kmalloc(dwrq->length, GFP_ATOMIC);
+   wbuff = kmalloc(dwrq->length, GFP_ATOMIC);
if (!wbuff)
return -EFAULT;
memcpy(wbuff, extra, dwrq->length);
-- 
2.7.4

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


[PATCH 05/12] staging: ks7010: remove dead code

2017-03-13 Thread Tobin C. Harding
Checkpatch emits CHECK: Alignment should match open parenthesis. This
is due to commented out code.

Remove commented out (dead) code.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index bae4011..fdacdae 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1719,7 +1719,6 @@ static int ks_wlan_get_scan(struct net_device *dev,
return -E2BIG;
}
current_ev = ks_wlan_translate_scan(dev, current_ev,
-//  extra + IW_SCAN_MAX_DATA,
extra + dwrq->length,
>current_ap);
}
@@ -1732,7 +1731,6 @@ static int ks_wlan_get_scan(struct net_device *dev,
}
/* Translate to WE format this entry */
current_ev = ks_wlan_translate_scan(dev, info, current_ev,
-//  extra + IW_SCAN_MAX_DATA,
extra + dwrq->length,
>aplist.ap[i]);
}
-- 
2.7.4

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


[PATCH 02/12] staging: ks7010: fix checkpatch BLOCK_COMMENT_STYLE

2017-03-13 Thread Tobin C. Harding
Checkpatch emits block comments warnings.

Change comments blocks to be inline with kernel coding style for
networking code.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 51 ++--
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index 75e42a7..1ff1948 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -230,7 +230,8 @@ static int ks_wlan_set_freq(struct net_device *dev,
} else {
int channel = fwrq->m;
/* We should do a better check than that,
-* based on the card capability !!! */
+* based on the card capability !!!
+*/
if ((channel < 1) || (channel > 14)) {
netdev_dbg(dev,
   "%s: New channel value of %d is invalid!\n",
@@ -343,7 +344,8 @@ static int ks_wlan_get_essid(struct net_device *dev,
 
/* for SLEEP MODE */
/* Note : if dwrq->flags != 0, we should
-* get the relevant SSID from the SSID list... */
+* get the relevant SSID from the SSID list...
+*/
if (priv->reg.ssid.size) {
/* Get the current SSID */
memcpy(extra, priv->reg.ssid.body, priv->reg.ssid.size);
@@ -1139,7 +1141,8 @@ static int ks_wlan_get_range(struct net_device *dev,
range->max_nwid = 0x;
range->num_channels = 14;
/* Should be based on cap_rid.country to give only
-* what the current card support */
+* what the current card support
+*/
k = 0;
for (i = 0; i < 13; i++) {  /* channel 1 -- 13 */
range->freq[k].i = i + 1;   /* List index */
@@ -1191,7 +1194,8 @@ static int ks_wlan_get_range(struct net_device *dev,
 
/* Set an indication of the max TCP throughput
 * in bit/s that we can expect using this interface.
-* May be use for QoS stuff... Jean II */
+* May be use for QoS stuff... Jean II
+*/
if (i > 2)
range->throughput = 5000 * 1000;
else
@@ -1225,9 +1229,11 @@ static int ks_wlan_get_range(struct net_device *dev,
range->retry_flags = IW_RETRY_ON;
range->r_time_flags = IW_RETRY_ON;
 
-   /* Experimental measurements - boundary 11/5.5 Mb/s */
-   /* Note : with or without the (local->rssi), results
-* are somewhat different. - Jean II */
+   /* Experimental measurements - boundary 11/5.5 Mb/s
+*
+* Note : with or without the (local->rssi), results
+* are somewhat different. - Jean II
+*/
range->avg_qual.qual = 50;
range->avg_qual.level = 186;/* -70 dBm */
range->avg_qual.noise = 0;
@@ -1500,7 +1506,8 @@ static inline char *ks_wlan_translate_scan(struct 
net_device *dev,
 ap->ssid.body);
 
/* Rate : stuffing multiple values in a single event require a bit
-* more of magic - Jean II */
+* more of magic - Jean II
+*/
current_val = current_ev + IW_EV_LCP_LEN;
 
iwe.cmd = SIOCGIWRATE;
@@ -1572,7 +1579,8 @@ static inline char *ks_wlan_translate_scan(struct 
net_device *dev,
}
 
/* The other data in the scan result are not really
-* interesting, so for now drop it - Jean II */
+* interesting, so for now drop it - Jean II
+*/
return current_ev;
 }
 
@@ -1599,7 +1607,8 @@ static int ks_wlan_get_scan(struct net_device *dev,
 
if (priv->aplist.size == 0) {
/* Client error, no scan results...
-* The caller need to restart the scan. */
+* The caller need to restart the scan.
+*/
DPRINTK(2, "aplist 0\n");
return -ENODATA;
}
@@ -1974,12 +1983,13 @@ static int ks_wlan_get_encode_ext(struct net_device 
*dev,
return -EPERM;
 
/* for SLEEP MODE */
-   /*  WPA (not used ?? wpa_supplicant)
-  struct ks_wlan_private *priv = (struct ks_wlan_private *)dev->priv;
-  struct iw_encode_ext *enc;
-  enc = (struct iw_encode_ext *)extra;
-  int index = dwrq->flags & IW_ENCODE_INDEX;
-  WPA (not used ?? wpa_supplicant) */
+   /* WPA (not used ?? wpa_supplicant)
+* struct ks_wlan_private *priv = (struct ks_wlan_private *)dev->priv;
+* struct iw_encode_ext *enc;
+* enc = (struct iw_encode_ext *)extra;
+* int index = dwrq->flags & IW_ENCODE_INDEX;
+* WPA (not used ?? wpa_supplicant)
+*/
return 0;
 }
 
@@ -2109,7 +2119,8 @@ static struct iw_statistics *ks_get_wireless_stats(struct 
net_device *dev)
}
 
/* Packets discarded in the wireless adapter due to wireless
-* specific problems */

[PATCH 01/12] staging: ks7010: fix checkpatch SPACING

2017-03-13 Thread Tobin C. Harding
Checkpatch emits over 100 instances of CHECK: No space is necessary
after a cast.

Remove unnecessary space.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 212 +--
 1 file changed, 106 insertions(+), 106 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index 2b4d6c1..75e42a7 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -238,7 +238,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
rc = -EINVAL;
} else {
/* Yes ! We can set it !!! */
-   priv->reg.channel = (u8) (channel);
+   priv->reg.channel = (u8)(channel);
priv->need_commit |= SME_MODE_SET;
}
}
@@ -493,12 +493,12 @@ static int ks_wlan_set_rate(struct net_device *dev,
case 1100:
case 550:
priv->reg.rate_set.body[0] =
-   (uint8_t) (vwrq->value / 50);
+   (uint8_t)(vwrq->value / 50);
break;
case 200:
case 100:
priv->reg.rate_set.body[0] =
-   ((uint8_t) (vwrq->value / 50)) |
+   ((uint8_t)(vwrq->value / 50)) |
BASIC_RATE;
break;
default:
@@ -550,7 +550,7 @@ static int ks_wlan_set_rate(struct net_device *dev,
case 1800:
case 900:
priv->reg.rate_set.body[0] =
-   (uint8_t) (vwrq->value / 50);
+   (uint8_t)(vwrq->value / 50);
break;
case 2400:
case 1200:
@@ -560,7 +560,7 @@ static int ks_wlan_set_rate(struct net_device *dev,
case 200:
case 100:
priv->reg.rate_set.body[0] =
-   ((uint8_t) (vwrq->value / 50)) |
+   ((uint8_t)(vwrq->value / 50)) |
BASIC_RATE;
break;
default:
@@ -3127,119 +3127,119 @@ static const struct iw_priv_args 
ks_wlan_private_args[] = {
 };
 
 static const iw_handler ks_wlan_handler[] = {
-   (iw_handler) ks_wlan_config_commit, /* SIOCSIWCOMMIT */
-   (iw_handler) ks_wlan_get_name,  /* SIOCGIWNAME */
-   (iw_handler) NULL,  /* SIOCSIWNWID */
-   (iw_handler) NULL,  /* SIOCGIWNWID */
-   (iw_handler) ks_wlan_set_freq,  /* SIOCSIWFREQ */
-   (iw_handler) ks_wlan_get_freq,  /* SIOCGIWFREQ */
-   (iw_handler) ks_wlan_set_mode,  /* SIOCSIWMODE */
-   (iw_handler) ks_wlan_get_mode,  /* SIOCGIWMODE */
+   (iw_handler)ks_wlan_config_commit,  /* SIOCSIWCOMMIT */
+   (iw_handler)ks_wlan_get_name,   /* SIOCGIWNAME */
+   (iw_handler)NULL,   /* SIOCSIWNWID */
+   (iw_handler)NULL,   /* SIOCGIWNWID */
+   (iw_handler)ks_wlan_set_freq,   /* SIOCSIWFREQ */
+   (iw_handler)ks_wlan_get_freq,   /* SIOCGIWFREQ */
+   (iw_handler)ks_wlan_set_mode,   /* SIOCSIWMODE */
+   (iw_handler)ks_wlan_get_mode,   /* SIOCGIWMODE */
 #ifndef KSC_OPNOTSUPP
-   (iw_handler) ks_wlan_set_sens,  /* SIOCSIWSENS */
-   (iw_handler) ks_wlan_get_sens,  /* SIOCGIWSENS */
+   (iw_handler)ks_wlan_set_sens,   /* SIOCSIWSENS */
+   (iw_handler)ks_wlan_get_sens,   /* SIOCGIWSENS */
 #else /* KSC_OPNOTSUPP */
-   (iw_handler) NULL,  /* SIOCSIWSENS */
-   (iw_handler) NULL,  /* SIOCGIWSENS */
+   (iw_handler)NULL,   /* SIOCSIWSENS */
+   (iw_handler)NULL,   /* SIOCGIWSENS */
 #endif /* KSC_OPNOTSUPP */
-   (iw_handler) NULL,  /* SIOCSIWRANGE */
-   (iw_handler) ks_wlan_get_range, /* SIOCGIWRANGE */
-   (iw_handler) NULL,  /* SIOCSIWPRIV */
-   (iw_handler) NULL,  /* SIOCGIWPRIV */
-   (iw_handler) NULL,  /* SIOCSIWSTATS */
-   (iw_handler) ks_wlan_get_iwstats,   /* SIOCGIWSTATS */
-   (iw_handler) NULL,  /* SIOCSIWSPY */
-   (iw_handler) NULL,  /* SIOCGIWSPY */
-   (iw_handler) NULL,  /* SIOCSIWTHRSPY */
-   (iw_handler) NULL,  /* SIOCGIWTHRSPY */
-   (iw_handler) ks_wlan_set_wap,   /* SIOCSIWAP */
-   (iw_handler) ks_wlan_get_wap,   /* SIOCGIWAP */
-//  (iw_handler) NULL,  /* SIOCSIWMLME */
-   (iw_handler) ks_wlan_set_mlme,  /* SIOCSIWMLME */
-   (iw_handler) 

[PATCH 00/12] staging: ks7010: fix checkpatch ks_wlan_net.c

2017-03-13 Thread Tobin C. Harding
Checkpatch emits various checks, warnings, and errors when parsing
ks_wlan_net.c.

Patch 01 fixes spacing issues.

Patch 02 converts block comments to use networking style block
comments.

Patch 03 converts function comments to use kernel doc format.

Patch 04 moves logical operators to the end of the previous line.

Patch 05 removes commented out code (dead code).

Patch 06 removes multiple assignment on single line.

Patch 07 moves comparison constant to the right hand side.

Patch 08 removes unnecessary else statements.

Patch 09 removes unnecessary cast from kmalloc() call.

Patch 10 replaces single byte memset with assignment.

Patch 11 reduces the level of indentation multiple times.

This patch is the meat of the patch set. I was not able to easily
visually parse the diff, admittedly I do not have a maintainers skill
at this but if there is an easier way to split this patch up I'm happy
to re-work it. First I tried reducing one level of indentation per
patch but then found there was no easy way to uniquely describe each
patch.

Patch breaks out of case block in multiple places - is
this ok? The result of this patch is less indentation, enhanced
readability, and most importantly opens up the way to refactoring that
fixes multiple types and instances of checkpatch warnings.

Multiple break statements are intermingled, breaking out of loops and
case blocks. For this reason a comment string was added to each
'break' in similar fashion to that used on #endif preprocessor
directives.

Patch 12 does whitespace refactor taking advantage of changes made in
the previous patch.

Code has not been tested. Series has been run through checkpatch
--strict, Smatch, and Sparse and does not introduce any new
errors. Each patch in series has been applied and built on x86_64 and
PowerPC. 

Tobin C. Harding (12):
  staging: ks7010: fix checkpatch SPACING
  staging: ks7010: fix checkpatch BLOCK_COMMENT_STYLE
  staging: ks7010: convert comments to kernel doc format
  staging: ks7010: fix logical line continuation
  staging: ks7010: remove dead code
  staging: ks7010: remove multiple assignment
  staging: ks7010: move comparison to right hand side
  staging: ks7010: remove unnecessary else statement
  staging: ks7010: remove unnecessary cast
  staging: ks7010: fix checkpatch memset warning
  staging: ks7010: reduce level of indentation
  staging: ks7010: refactor, whitespace only

 drivers/staging/ks7010/ks_wlan_net.c | 1040 +-
 1 file changed, 640 insertions(+), 400 deletions(-)

-- 
2.7.4

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


[PATCH 06/12] staging: ks7010: remove multiple assignment

2017-03-13 Thread Tobin C. Harding
Checkpatch emits CHECK: multiple assignments should be avoided.

Move multiple assignment onto separate lines. Fix comment to use more
natural English.

Signed-off-by: Tobin C. Harding 
---
 drivers/staging/ks7010/ks_wlan_net.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c 
b/drivers/staging/ks7010/ks_wlan_net.c
index fdacdae..59eabb9 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1607,8 +1607,10 @@ static inline char *ks_wlan_translate_scan(struct 
net_device *dev,
current_val = current_ev + IW_EV_LCP_LEN;
 
iwe.cmd = SIOCGIWRATE;
-   /* Those two flags are ignored... */
-   iwe.u.bitrate.fixed = iwe.u.bitrate.disabled = 0;
+
+   /* These two flags are ignored... */
+   iwe.u.bitrate.fixed = 0;
+   iwe.u.bitrate.disabled = 0;
 
/* Max 16 values */
for (i = 0; i < 16; i++) {
-- 
2.7.4

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


Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

2017-03-13 Thread Okash Khawaja
On Tue, Mar 14, 2017 at 06:12:47AM +0800, Greg Kroah-Hartman wrote:
> On Mon, Mar 13, 2017 at 10:05:52PM +, okash.khaw...@gmail.com wrote:
> > Allow access to TTY device from kernel. This is based on Alan Cox's patch
> > (http://www.mail-archive.com/linux-kernel at 
> > vger.kernel.org/msg1215095.html),
> > with description quoted below.
> > 
> > "tty_port: allow a port to be opened with a tty that has no file handle
> > 
> > Let us create tty objects entirely in kernel space.
> > 
> > With this a kernel created non file backed tty object could be used to 
> > handle
> > data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> > particular has to work back to the fs/tty layer.
> > 
> > The tty_port code is however otherwise clean of file handles as far as I can
> > tell as is the low level tty port write path used by the ldisc, the
> > configuration low level interfaces and most of the ldiscs.
> > 
> > Currently you don't have any exposure to see tty hangups because those are
> > built around the file layer. However a) it's a fixed port so you probably
> > don't care about that b) if you do we can add a callback and c) you almost
> > certainly don't want the userspace tear down/rebuild behaviour anyway.
> > 
> > This should however be sufficient if we wanted for example to enumerate all
> > the bluetooth bound fixed ports via ACPI and make them directly available.
> > 
> > It doesn't deal with the case of a user opening a port that's also kernel
> > opened and that would need some locking out (so it returned EBUSY if bound
> > to a kernel device of some kind). That needs resolving along with how you
> > "up" or "down" your new bluetooth device, or enumerate it while providing
> > the existing tty API to avoid regressions (and to debug)."
> > 
> > Signed-off-by: Okash Khawaja 
> > 
> > Reviewed-by: Samuel Thibault 
> 
> You do know this is already in 4.11-rc1, right?  Please rebase your
> patch set on 4.11-rc2 at the least and resend.

Didn't realise that! Will resend
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Samuel Thibault
Hello,

Greg KH, on mar. 14 mars 2017 06:14:04 +0800, wrote:
> On Mon, Mar 13, 2017 at 10:05:51PM +, okash.khaw...@gmail.com wrote:
> > This patchset introduces a TTY-based way for the synths to communicate
> > with devices as an alternate for direct serial comms used by the synths
> > at the moment. It then migrates some of the synths to the TTY-based
> > comms. Synths migrated in this patchset are dummy, acntsa, bns and
> > txprt.
> 
> What about using the serbus code that is now in the tree?

I guess you mean serdev?

Well, for a start that didn't exist when Okash worked on the TTY part :)

> That should make this a lot easier than your patchset from what I can
> see.

Will serdev support modem line control?  We need this for some devices.
How is the serial port chosen with serdev?

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


Re: [PATCH 3/3] staging: dngc: ch->ch_bd is already assigned to bd variable

2017-03-13 Thread Greg KH
On Sun, Mar 12, 2017 at 11:47:28PM +0900, Daeseok Youn wrote:
> The bd variable in dgnc_tty_digiseta() is assigned with
> ch->ch_bd but it is not used in this function except checking NULL.
> The ch->ch_bd could be replaced with bd variable in dgnc_tty_digiseta()
> 
> Signed-off-by: Daeseok Youn 
> ---
>  drivers/staging/dgnc/dgnc_tty.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please merge this with your first patch in the series and resend them.

thanks,

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


Re: [PATCH] staging: ks7010: fix spelling of Michael MIC

2017-03-13 Thread Greg Kroah-Hartman
On Mon, Mar 13, 2017 at 08:39:29PM +1100, Tobin C. Harding wrote:
> Driver mixes spelling michael and michel in symbol names and
> comments. Michael here references the IEEE 802.11i Message Integrity
> Code. It is incorrect to spell it michel and confusing having two
> spellings for the same thing.
> 
> Change michel -> micheal in both symbol names and comments.
> 
> Signed-off-by: Tobin C. Harding 
> ---
>  drivers/staging/ks7010/ks_hostif.c   | 16 
>  drivers/staging/ks7010/michael_mic.c |  8 
>  drivers/staging/ks7010/michael_mic.h |  6 +++---
>  3 files changed, 15 insertions(+), 15 deletions(-)

This patch does not apply to my staging-testing branch :(

thanks,

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


Re: [PATCH 0/2] staging: ks7010: fix checkpatch ks_wlan_ioctl.h

2017-03-13 Thread Greg Kroah-Hartman
On Tue, Mar 14, 2017 at 06:18:48AM +0800, Greg Kroah-Hartman wrote:
> On Tue, Mar 14, 2017 at 09:10:18AM +1100, Tobin C. Harding wrote:
> > On Mon, Mar 13, 2017 at 06:54:06PM +1100, Tobin C. Harding wrote:
> > > Simple patch set. Clears 29 checkpatch errors/warnings.
> > > 
> > > Patch 01 does various fixes, all whitespace only changes.
> > > 
> > > Is this ok putting multiple checkpatch types in one patch if they are
> > > all whitespace? Please advise, I would like to make review as easy as
> > > possible. 
> > > 
> > > Patch 02 adds parentheses to arithmetic macro definitions (24
> > > identical cases).
> > > 
> > > Tobin C. Harding (2):
> > >   staging: ks7010: fix checkpatch whitespace warns
> > >   staging: ks7010: add parentheses to complex macro
> > > 
> > >  drivers/staging/ks7010/ks_wlan_ioctl.h | 68 
> > > +-
> > >  1 file changed, 34 insertions(+), 34 deletions(-)
> > > 
> > > -- 
> > > 2.7.4
> > > 
> > 
> > Drop this patch set please. I see Matthew Giassa started work on this
> > file before me.
> 
> I don't see those patches still in my queue. Is there anything wrong
> with this series that would prevent it from being acceptable?

In fact, they look fine to me, now queued up.

thanks,

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


Re: [Outreachy kernel] [PATCH v2] staging: sm750fb: Improved code readability

2017-03-13 Thread Julia Lawall


On Mon, 13 Mar 2017, Arushi Singhal wrote:

> New variables are added to make the code more readable.
>
> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - removed the error.
>
>  drivers/staging/sm750fb/ddk750_mode.c | 103 
> --
>  1 file changed, 49 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
> b/drivers/staging/sm750fb/ddk750_mode.c
> index 25da678179f7..6dd52b450367 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -77,37 +77,34 @@ static int programModeRegisters(struct _mode_parameter_t 
> *pModeParam, struct pll
>   int ret = 0;
>   int cnt = 0;
>   unsigned int tmp, reg;
> + unsigned int cht = CRT_HORIZONTAL_TOTAL;
> + unsigned int cvt = CRT_VERTICAL_TOTAL;
> + unsigned int chs = CRT_HORIZONTAL_SYNC;
> + unsigned int cvs = CRT_VERTICAL_SYNC;
> + unsigned int chssm = CRT_HORIZONTAL_SYNC_START_MASK;
> + unsigned int cvssm = CRT_VERTICAL_SYNC_START_MASK;
> + unsigned int chtdem = CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;
> + unsigned int cvtdem = CRT_VERTICAL_TOTAL_DISPLAY_END_MASK;
> + unsigned int chttm = CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
> + unsigned int cvttm = CRT_VERTICAL_TOTAL_TOTAL_MASK;
> + unsigned int chswm = CRT_HORIZONTAL_SYNC_WIDTH_MASK;
> + unsigned int cvshm = CRT_VERTICAL_SYNC_HEIGHT_MASK;
> + unsigned int phde = pModeParam->horizontal_display_end - 1;
> + unsigned int pvde = pModeParam->vertical_display_end - 1;
> + unsigned int phss = pModeParam->horizontal_sync_start - 1;
> + unsigned int pvss = pModeParam->vertical_sync_start - 1;

I dont think this is a good solution.  The new names are not very
memorable, and they are not used near their definition.  You could instead
name a subexpression, and put the initialization right next to where the
variable would be used.  So for a trivial example, you could turn

x = (a + b) * c;

into

tmp = a * b;
x = tmp * c;

What you have in the definition of tmp should do some computation but
be able to be laid out in a readable manner.

julia

>
>   if (pll->clockType == SECONDARY_PLL) {
>   /* programe secondary pixel clock */
>   poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
> - poke32(CRT_HORIZONTAL_TOTAL,
> -(((pModeParam->horizontal_total - 1) <<
> -  CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
> - CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
> -((pModeParam->horizontal_display_end - 1) &
> - CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
> -
> - poke32(CRT_HORIZONTAL_SYNC,
> -((pModeParam->horizontal_sync_width <<
> -  CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
> - CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
> -((pModeParam->horizontal_sync_start - 1) &
> - CRT_HORIZONTAL_SYNC_START_MASK));
> -
> - poke32(CRT_VERTICAL_TOTAL,
> -(((pModeParam->vertical_total - 1) <<
> -  CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
> - CRT_VERTICAL_TOTAL_TOTAL_MASK) |
> -((pModeParam->vertical_display_end - 1) &
> - CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
> -
> - poke32(CRT_VERTICAL_SYNC,
> -((pModeParam->vertical_sync_height <<
> -  CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
> - CRT_VERTICAL_SYNC_HEIGHT_MASK) |
> -((pModeParam->vertical_sync_start - 1) &
> - CRT_VERTICAL_SYNC_START_MASK));
> +
> + poke32(cht, (((pModeParam->horizontal_total - 1) << 
> CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) & chttm) | (phde & chtdem));
> +
> + poke32(chs, ((pModeParam->horizontal_sync_width << 
> CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) & chswm) | (phss & chssm));
> +
> + poke32(cvt, (((pModeParam->vertical_total - 1) << 
> CRT_VERTICAL_TOTAL_TOTAL_SHIFT) & cvttm) | (pvde & cvtdem));
> +
> + poke32(cvs, ((pModeParam->vertical_sync_height << 
> CRT_VERTICAL_SYNC_HEIGHT_SHIFT) & cvshm) | (pvss & cvssm));
>
>   tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
>   if (pModeParam->vertical_sync_polarity)
> @@ -128,36 +125,34 @@ static int programModeRegisters(struct 
> _mode_parameter_t *pModeParam, struct pll
>
>   } else if (pll->clockType == PRIMARY_PLL) {
>   unsigned int reserved;
> + unsigned int pht = PANEL_HORIZONTAL_TOTAL;
> + unsigned int pvt = PANEL_VERTICAL_TOTAL;
> + unsigned int phs = PANEL_HORIZONTAL_SYNC;
> + unsigned int pvs = PANEL_VERTICAL_SYNC;
> + unsigned int phssm = PANEL_HORIZONTAL_SYNC_START_MASK;
> + unsigned int pvssm = PANEL_VERTICAL_SYNC_START_MASK;
> +

Re: [PATCH 0/2] staging: ks7010: fix checkpatch ks_wlan_ioctl.h

2017-03-13 Thread Greg Kroah-Hartman
On Tue, Mar 14, 2017 at 09:10:18AM +1100, Tobin C. Harding wrote:
> On Mon, Mar 13, 2017 at 06:54:06PM +1100, Tobin C. Harding wrote:
> > Simple patch set. Clears 29 checkpatch errors/warnings.
> > 
> > Patch 01 does various fixes, all whitespace only changes.
> > 
> > Is this ok putting multiple checkpatch types in one patch if they are
> > all whitespace? Please advise, I would like to make review as easy as
> > possible. 
> > 
> > Patch 02 adds parentheses to arithmetic macro definitions (24
> > identical cases).
> > 
> > Tobin C. Harding (2):
> >   staging: ks7010: fix checkpatch whitespace warns
> >   staging: ks7010: add parentheses to complex macro
> > 
> >  drivers/staging/ks7010/ks_wlan_ioctl.h | 68 
> > +-
> >  1 file changed, 34 insertions(+), 34 deletions(-)
> > 
> > -- 
> > 2.7.4
> > 
> 
> Drop this patch set please. I see Matthew Giassa started work on this
> file before me.

I don't see those patches still in my queue. Is there anything wrong
with this series that would prevent it from being acceptable?

Multiple people working on the same file isn't a reason for one to
stop...

thanks,

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


Re: [patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread Greg Kroah-Hartman
On Mon, Mar 13, 2017 at 10:05:51PM +, okash.khaw...@gmail.com wrote:
> Hi,
> 
> This patchset introduces a TTY-based way for the synths to communicate
> with devices as an alternate for direct serial comms used by the synths
> at the moment. It then migrates some of the synths to the TTY-based
> comms. Synths migrated in this patchset are dummy, acntsa, bns and
> txprt.

What about using the serbus code that is now in the tree?  That should
make this a lot easier than your patchset from what I can see.

thanks,

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


Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

2017-03-13 Thread Greg Kroah-Hartman
On Mon, Mar 13, 2017 at 10:05:52PM +, okash.khaw...@gmail.com wrote:
> Allow access to TTY device from kernel. This is based on Alan Cox's patch
> (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
> with description quoted below.
> 
> "tty_port: allow a port to be opened with a tty that has no file handle
> 
> Let us create tty objects entirely in kernel space.
> 
> With this a kernel created non file backed tty object could be used to handle
> data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> particular has to work back to the fs/tty layer.
> 
> The tty_port code is however otherwise clean of file handles as far as I can
> tell as is the low level tty port write path used by the ldisc, the
> configuration low level interfaces and most of the ldiscs.
> 
> Currently you don't have any exposure to see tty hangups because those are
> built around the file layer. However a) it's a fixed port so you probably
> don't care about that b) if you do we can add a callback and c) you almost
> certainly don't want the userspace tear down/rebuild behaviour anyway.
> 
> This should however be sufficient if we wanted for example to enumerate all
> the bluetooth bound fixed ports via ACPI and make them directly available.
> 
> It doesn't deal with the case of a user opening a port that's also kernel
> opened and that would need some locking out (so it returned EBUSY if bound
> to a kernel device of some kind). That needs resolving along with how you
> "up" or "down" your new bluetooth device, or enumerate it while providing
> the existing tty API to avoid regressions (and to debug)."
> 
> Signed-off-by: Okash Khawaja 
> 
> Reviewed-by: Samuel Thibault 

You do know this is already in 4.11-rc1, right?  Please rebase your
patch set on 4.11-rc2 at the least and resend.

thanks,

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


Re: [PATCH 0/2] staging: ks7010: fix checkpatch ks_wlan_ioctl.h

2017-03-13 Thread Tobin C. Harding
On Mon, Mar 13, 2017 at 06:54:06PM +1100, Tobin C. Harding wrote:
> Simple patch set. Clears 29 checkpatch errors/warnings.
> 
> Patch 01 does various fixes, all whitespace only changes.
> 
> Is this ok putting multiple checkpatch types in one patch if they are
> all whitespace? Please advise, I would like to make review as easy as
> possible. 
> 
> Patch 02 adds parentheses to arithmetic macro definitions (24
> identical cases).
> 
> Tobin C. Harding (2):
>   staging: ks7010: fix checkpatch whitespace warns
>   staging: ks7010: add parentheses to complex macro
> 
>  drivers/staging/ks7010/ks_wlan_ioctl.h | 68 
> +-
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> -- 
> 2.7.4
> 

Drop this patch set please. I see Matthew Giassa started work on this
file before me.

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


[patch 2/7] staging: speakup: spk_serial_out and spk_wait_for_xmitr to take synth arg

2017-03-13 Thread okash . khawaja
These two functions are always called from a context where spk_synth instance
is available. They also use the spk_synth instance but instead of taking it
as an argument, they rely on a global spk_synth instance inside synth.c which
points to the same synth as the one being passed in as argument.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -144,14 +144,14 @@
free_irq(serstate->irq, (void *)synth_readbuf_handler);
 }
 
-int spk_wait_for_xmitr(void)
+int spk_wait_for_xmitr(struct spk_synth *in_synth)
 {
int tmout = SPK_XMITR_TIMEOUT;
 
-   if ((synth->alive) && (timeouts >= NUM_DISABLE_TIMEOUTS)) {
+   if ((in_synth->alive) && (timeouts >= NUM_DISABLE_TIMEOUTS)) {
pr_warn("%s: too many timeouts, deactivating speakup\n",
-   synth->long_name);
-   synth->alive = 0;
+   in_synth->long_name);
+   in_synth->alive = 0;
/* No synth any more, so nobody will restart TTYs, and we thus
 * need to do it ourselves.  Now that there is no synth we can
 * let application flood anyway
@@ -162,7 +162,7 @@
}
while (spk_serial_tx_busy()) {
if (--tmout == 0) {
-   pr_warn("%s: timed out (tx busy)\n", synth->long_name);
+   pr_warn("%s: timed out (tx busy)\n", 
in_synth->long_name);
timeouts++;
return 0;
}
@@ -207,9 +207,9 @@
 }
 EXPORT_SYMBOL_GPL(spk_serial_in_nowait);
 
-int spk_serial_out(const char ch)
+int spk_serial_out(struct spk_synth *in_synth, const char ch)
 {
-   if (synth->alive && spk_wait_for_xmitr()) {
+   if (in_synth->alive && spk_wait_for_xmitr(in_synth)) {
outb_p(ch, speakup_info.port_tts);
return 1;
}
Index: linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
@@ -169,7 +169,7 @@
set_current_state(TASK_INTERRUPTIBLE);
full_time_val = full_time->u.n.value;
spin_unlock_irqrestore(_info.spinlock, flags);
-   if (!spk_serial_out(ch)) {
+   if (!spk_serial_out(synth, ch)) {
outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);
@@ -182,7 +182,7 @@
full_time_val = full_time->u.n.value;
delay_time_val = delay_time->u.n.value;
spin_unlock_irqrestore(_info.spinlock, flags);
-   if (spk_serial_out(synth->procspeech))
+   if (spk_serial_out(synth, synth->procspeech))
schedule_timeout(msecs_to_jiffies
 (delay_time_val));
else
@@ -195,7 +195,7 @@
synth_buffer_getc();
spin_unlock_irqrestore(_info.spinlock, flags);
}
-   spk_serial_out(PROCSPEECH);
+   spk_serial_out(synth, PROCSPEECH);
 }
 
 module_param_named(ser, synth_apollo.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
@@ -135,7 +135,7 @@
udelay(1);
}
outb(SYNTH_CLEAR, speakup_info.port_tts);
-   spk_serial_out(PROCSPEECH);
+   spk_serial_out(synth, PROCSPEECH);
 }
 
 static void synth_version(struct spk_synth *synth)
Index: linux-4.10.1/drivers/staging/speakup/speakup_decext.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decext.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decext.c
@@ -186,7 +186,7 @@
spin_unlock_irqrestore(_info.spinlock, flags);
if (ch == '\n')
ch = 0x0D;
-   if (synth_full() || !spk_serial_out(ch)) {
+   if (synth_full() || !spk_serial_out(synth, ch)) {
schedule_timeout(msecs_to_jiffies(delay_time_val));
continue;
}
@@ -200,10 +200,10 @@
in_escape = 0;
else if (ch <= SPACE) {
if (!in_escape && 

[patch 0/7] staging: speakup: introduce tty-based comms

2017-03-13 Thread okash . khawaja
Hi,

This patchset introduces a TTY-based way for the synths to communicate
with devices as an alternate for direct serial comms used by the synths
at the moment. It then migrates some of the synths to the TTY-based
comms. Synths migrated in this patchset are dummy, acntsa, bns and
txprt.

As such, this series is meant to start a discussion on the changes in the
first patch: "tty_port: allow a port to be opened with a tty that has no
file handle". This allows speakup to access tty port early on during boot,
making it possible to start emiting messages as early as possible. This is
demonstrated in patch 6, where it calls tty_open_by_driver, passing in NULL
for inode and filp params.

To wrap up, here is a summary of the patches that follow. Patch 1 enables
kernel access to TTY device. Patches 2 - 5 prepare the code to accomodate
TTY-based comms as an alternate to raw serial I/O for outputting data to
external device. Patch 6 builds upon the changes made in patch 1 and utilises
TTY-subsystem to communicate with external device - currently just over ttyS*
but with plans to expand it to other TTY devices soon. Finally, patch 7 does
actual migration of some of the simple synths from raw serial comms to TTY.

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


[patch 4/7] staging: speakup: move spk_stop_serial_interrupt into synth-specific release function

2017-03-13 Thread okash . khawaja
This moves call to spk_stop_serial_interrupt() function out of synth_release()
and into release() method of specific spk_synth instances. This is because
a TTY-based synth implementation wouldn't need spk_stop_serial_interrupt()
call. Moving it into each synth's release() method gives the decision of
calling  spk_stop_serial_interrupt() to that synth. TTY-based synths which
follow in this patchset simply wouldn't call it.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -149,6 +149,7 @@
/* Free IRQ */
free_irq(serstate->irq, (void *)synth_readbuf_handler);
 }
+EXPORT_SYMBOL_GPL(spk_stop_serial_interrupt);
 
 int spk_wait_for_xmitr(struct spk_synth *in_synth)
 {
@@ -224,6 +225,7 @@
 
 void spk_serial_release(void)
 {
+   spk_stop_serial_interrupt();
if (speakup_info.port_tts == 0)
return;
synth_release_region(speakup_info.port_tts, 8);
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
@@ -304,6 +304,7 @@
 
 static void accent_release(void)
 {
+   spk_stop_serial_interrupt();
if (speakup_info.port_tts)
synth_release_region(speakup_info.port_tts-1, SYNTH_IO_EXTENT);
speakup_info.port_tts = 0;
Index: linux-4.10.1/drivers/staging/speakup/synth.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/synth.c
+++ linux-4.10.1/drivers/staging/speakup/synth.c
@@ -462,7 +462,6 @@
sysfs_remove_group(speakup_kobj, >attributes);
for (var = synth->vars; var->var_id != MAXVARS; var++)
speakup_unregister_var(var->var_id);
-   spk_stop_serial_interrupt();
synth->release();
synth = NULL;
 }
Index: linux-4.10.1/drivers/staging/speakup/speakup_decpc.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_decpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_decpc.c
@@ -483,6 +483,7 @@
 
 static void dtpc_release(void)
 {
+   spk_stop_serial_interrupt();
if (speakup_info.port_tts)
synth_release_region(speakup_info.port_tts, SYNTH_IO_EXTENT);
speakup_info.port_tts = 0;
Index: linux-4.10.1/drivers/staging/speakup/speakup_dtlk.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dtlk.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dtlk.c
@@ -375,6 +375,7 @@
 
 static void dtlk_release(void)
 {
+   spk_stop_serial_interrupt();
if (speakup_info.port_tts)
synth_release_region(speakup_info.port_tts-1, SYNTH_IO_EXTENT);
speakup_info.port_tts = 0;
Index: linux-4.10.1/drivers/staging/speakup/speakup_keypc.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_keypc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_keypc.c
@@ -306,6 +306,7 @@
 
 static void keynote_release(void)
 {
+   spk_stop_serial_interrupt();
if (synth_port)
synth_release_region(synth_port, SYNTH_IO_EXTENT);
synth_port = 0;

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


[patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

2017-03-13 Thread okash . khawaja
Allow access to TTY device from kernel. This is based on Alan Cox's patch
(http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
with description quoted below.

"tty_port: allow a port to be opened with a tty that has no file handle

Let us create tty objects entirely in kernel space.

With this a kernel created non file backed tty object could be used to handle
data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
particular has to work back to the fs/tty layer.

The tty_port code is however otherwise clean of file handles as far as I can
tell as is the low level tty port write path used by the ldisc, the
configuration low level interfaces and most of the ldiscs.

Currently you don't have any exposure to see tty hangups because those are
built around the file layer. However a) it's a fixed port so you probably
don't care about that b) if you do we can add a callback and c) you almost
certainly don't want the userspace tear down/rebuild behaviour anyway.

This should however be sufficient if we wanted for example to enumerate all
the bluetooth bound fixed ports via ACPI and make them directly available.

It doesn't deal with the case of a user opening a port that's also kernel
opened and that would need some locking out (so it returned EBUSY if bound
to a kernel device of some kind). That needs resolving along with how you
"up" or "down" your new bluetooth device, or enumerate it while providing
the existing tty API to avoid regressions (and to debug)."

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/tty/tty_io.c
===
--- linux-4.10.1.orig/drivers/tty/tty_io.c
+++ linux-4.10.1/drivers/tty/tty_io.c
@@ -855,7 +855,7 @@
 
 int tty_hung_up_p(struct file *filp)
 {
-   return (filp->f_op == _up_tty_fops);
+   return (filp && filp->f_op == _up_tty_fops);
 }
 
 EXPORT_SYMBOL(tty_hung_up_p);
@@ -1368,7 +1368,10 @@
struct tty_struct *tty;
 
if (driver->ops->lookup)
-   tty = driver->ops->lookup(driver, file, idx);
+   if (!file)
+   tty = ERR_PTR(-EIO);
+   else
+   tty = driver->ops->lookup(driver, file, idx);
else
tty = driver->ttys[idx];
 
@@ -1986,7 +1989,7 @@
struct tty_driver *console_driver = console_device(index);
if (console_driver) {
driver = tty_driver_kref_get(console_driver);
-   if (driver) {
+   if (driver && filp) {
/* Don't let /dev/console block */
filp->f_flags |= O_NONBLOCK;
break;
@@ -2019,7 +2022,7 @@
  *   - concurrent tty driver removal w/ lookup
  *   - concurrent tty removal from driver table
  */
-static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
 struct file *filp)
 {
struct tty_struct *tty;
@@ -2064,6 +2067,7 @@
tty_driver_kref_put(driver);
return tty;
 }
+EXPORT_SYMBOL(tty_open_by_driver);
 
 /**
  * tty_open-   open a tty device
Index: linux-4.10.1/drivers/tty/tty_port.c
===
--- linux-4.10.1.orig/drivers/tty/tty_port.c
+++ linux-4.10.1/drivers/tty/tty_port.c
@@ -335,7 +335,7 @@
  * tty_port_block_til_ready-   Waiting logic for tty open
  * @port: the tty port being opened
  * @tty: the tty device being bound
- * @filp: the file pointer of the opener
+ * @filp: the file pointer of the opener or NULL
  *
  * Implement the core POSIX/SuS tty behaviour when opening a tty device.
  * Handles:
@@ -369,7 +369,7 @@
tty_port_set_active(port, 1);
return 0;
}
-   if (filp->f_flags & O_NONBLOCK) {
+   if (filp ==  NULL || filp->f_flags & O_NONBLOCK) {
/* Indicate we are open */
if (C_BAUD(tty))
tty_port_raise_dtr_rts(port);
Index: linux-4.10.1/include/linux/tty.h
===
--- linux-4.10.1.orig/include/linux/tty.h
+++ linux-4.10.1/include/linux/tty.h
@@ -394,6 +394,8 @@
 /* tty_io.c */
 extern int __init tty_init(void);
 extern const char *tty_name(const struct tty_struct *tty);
+extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+struct file *filp);
 #else
 static inline void console_init(void)
 { }

___
devel mailing list
de...@linuxdriverproject.org

[patch 6/7] staging: speakup: add tty-based comms functions

2017-03-13 Thread okash . khawaja
This adds spk_ttyio.c file. It contains a set of functions which implement
those methods in spk_synth struct which relate to sending bytes out using
serial comms. Implementations in this file perform the same function but
using TTY subsystem instead. Currently synths access serial ports, directly
poking standard ISA ports by trying to steal them from serial driver. Some ISA
cards actually need this way of doing it, but most other synthesizers don't,
and can actually work by using the proper TTY subsystem through a new N_SPEAKUP
line discipline. So this adds the methods for drivers to switch to accessing
serial ports through the TTY subsystem, whenever appropriate.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/staging/speakup/Makefile
===
--- linux-4.10.1.orig/drivers/staging/speakup/Makefile
+++ linux-4.10.1/drivers/staging/speakup/Makefile
@@ -25,6 +25,7 @@
kobjects.o \
selection.o \
serialio.o \
+   spk_ttyio.o \
synth.o \
thread.o \
varhandlers.o
Index: linux-4.10.1/drivers/staging/speakup/spk_priv.h
===
--- linux-4.10.1.orig/drivers/staging/speakup/spk_priv.h
+++ linux-4.10.1/drivers/staging/speakup/spk_priv.h
@@ -46,6 +46,7 @@
 unsigned char spk_serial_in(void);
 unsigned char spk_serial_in_nowait(void);
 void spk_serial_release(void);
+void spk_ttyio_release(void);
 
 void synth_buffer_skip_nonlatin1(void);
 u16 synth_buffer_getc(void);
@@ -58,7 +59,9 @@
  const char *buf, size_t count);
 
 int spk_serial_synth_probe(struct spk_synth *synth);
+int spk_ttyio_synth_probe(struct spk_synth *synth);
 const char *spk_serial_synth_immediate(struct spk_synth *synth, const char 
*buff);
+const char *spk_ttyio_synth_immediate(struct spk_synth *synth, const char 
*buff);
 void spk_do_catch_up(struct spk_synth *synth);
 void spk_synth_flush(struct spk_synth *synth);
 int spk_synth_is_alive_nop(struct spk_synth *synth);
@@ -78,5 +81,6 @@
 extern struct var_t synth_time_vars[];
 
 extern struct spk_io_ops spk_serial_io_ops;
+extern struct spk_io_ops spk_ttyio_ops;
 
 #endif
Index: linux-4.10.1/drivers/staging/speakup/spk_ttyio.c
===
--- /dev/null
+++ linux-4.10.1/drivers/staging/speakup/spk_ttyio.c
@@ -0,0 +1,143 @@
+#include 
+#include 
+
+#include "speakup.h"
+#include "spk_types.h"
+
+static struct tty_struct *speakup_tty;
+
+static int spk_ttyio_ldisc_open(struct tty_struct *tty)
+{
+   if (tty->ops->write == NULL)
+   return -EOPNOTSUPP;
+   speakup_tty = tty;
+
+   return 0;
+}
+
+static void spk_ttyio_ldisc_close(struct tty_struct *tty)
+{
+   speakup_tty = NULL;
+}
+
+static struct tty_ldisc_ops spk_ttyio_ldisc_ops = {
+   .owner  = THIS_MODULE,
+   .magic  = TTY_LDISC_MAGIC,
+   .name   = "speakup_ldisc",
+   .open   = spk_ttyio_ldisc_open,
+   .close  = spk_ttyio_ldisc_close,
+};
+
+static int spk_ttyio_out(struct spk_synth *in_synth, const char ch);
+struct spk_io_ops spk_ttyio_ops = {
+   .synth_out = spk_ttyio_out,
+};
+EXPORT_SYMBOL_GPL(spk_ttyio_ops);
+
+static int spk_ttyio_initialise_ldisc(int ser)
+{
+   int ret = 0;
+   struct tty_struct *tty;
+
+   ret = tty_register_ldisc(N_SPEAKUP, _ttyio_ldisc_ops);
+   if (ret) {
+   pr_err("Error registering line discipline.\n");
+   return ret;
+   }
+
+   if (ser < 0 || ser > (255 - 64)) {
+   pr_err("speakup: Invalid ser param. Must be between 0 and 191 
inclusive.\n");
+   return -EINVAL;
+   }
+
+   /* TODO: support more than ttyS* */
+   tty = tty_open_by_driver(MKDEV(4, (ser +  64)), NULL, NULL);
+   if (IS_ERR(tty))
+   return PTR_ERR(tty);
+
+   if (tty->ops->open)
+   ret = tty->ops->open(tty, NULL);
+   else
+   ret = -ENODEV;
+
+   if (ret) {
+   tty_unlock(tty);
+   return ret;
+   }
+
+   clear_bit(TTY_HUPPED, >flags);
+   tty_unlock(tty);
+
+   ret = tty_set_ldisc(tty, N_SPEAKUP);
+
+   return ret;
+}
+
+static int spk_ttyio_out(struct spk_synth *in_synth, const char ch)
+{
+   if (in_synth->alive && speakup_tty && speakup_tty->ops->write) {
+   int ret = speakup_tty->ops->write(speakup_tty, , 1);
+   if (ret == 0)
+   /* No room */
+   return 0;
+   if (ret < 0) {
+   pr_warn("%s: I/O error, deactivating speakup\n", 
in_synth->long_name);
+   /* No synth any more, so nobody will restart TTYs, and 
we thus
+* need to do it ourselves.  Now that there is no synth 
we can
+  

[patch 3/7] staging: serial: add spk_io_ops struct to spk_synth

2017-03-13 Thread okash . khawaja
This patch adds spk_io_ops struct which contain those methods whose job is to
communicate with synth device. Currently, all comms with external synth
device use raw serial i/o. Starting with this patch set, an alternative
tty-based way of communication is being introduced. The idea is to group all
methods which do the actual communication with external device into this new
struct. Then migrating a serial-based synth over to tty will mean swapping
serial spk_io_ops instance with tty spk_io_ops instance, making the migration
simpler.

At the moment, this struct only contains one method, synth_out but more will
be added in future when incorporating inputs in the tty-based comms. Also at
the moment, synth_out method has one implementation which uses serial i/o.
Later in this patch set, an alternate tty-based implementation will be added.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -24,6 +24,12 @@
 static const struct old_serial_port *serstate;
 static int timeouts;
 
+static int spk_serial_out(struct spk_synth *in_synth, const char ch);
+struct spk_io_ops spk_serial_io_ops = {
+   .synth_out = spk_serial_out,
+};
+EXPORT_SYMBOL_GPL(spk_serial_io_ops);
+
 const struct old_serial_port *spk_serial_init(int index)
 {
int baud = 9600, quot = 0;
@@ -215,7 +221,6 @@
}
return 0;
 }
-EXPORT_SYMBOL_GPL(spk_serial_out);
 
 void spk_serial_release(void)
 {
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntpc.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntpc.c
@@ -113,6 +113,7 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
+   .io_ops = _serial_io_ops,
.probe = synth_probe,
.release = accent_release,
.synth_immediate = synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
@@ -99,6 +99,7 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
+   .io_ops = _serial_io_ops,
.probe = synth_probe,
.release = spk_serial_release,
.synth_immediate = spk_synth_immediate,
Index: linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
@@ -108,6 +108,7 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
+   .io_ops = _serial_io_ops,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
.synth_immediate = spk_synth_immediate,
@@ -169,7 +170,7 @@
set_current_state(TASK_INTERRUPTIBLE);
full_time_val = full_time->u.n.value;
spin_unlock_irqrestore(_info.spinlock, flags);
-   if (!spk_serial_out(synth, ch)) {
+   if (!synth->io_ops->synth_out(synth, ch)) {
outb(UART_MCR_DTR, speakup_info.port_tts + UART_MCR);
outb(UART_MCR_DTR | UART_MCR_RTS,
speakup_info.port_tts + UART_MCR);
@@ -182,7 +183,7 @@
full_time_val = full_time->u.n.value;
delay_time_val = delay_time->u.n.value;
spin_unlock_irqrestore(_info.spinlock, flags);
-   if (spk_serial_out(synth, synth->procspeech))
+   if (synth->io_ops->synth_out(synth, synth->procspeech))
schedule_timeout(msecs_to_jiffies
 (delay_time_val));
else
@@ -195,7 +196,7 @@
synth_buffer_getc();
spin_unlock_irqrestore(_info.spinlock, flags);
}
-   spk_serial_out(synth, PROCSPEECH);
+   synth->io_ops->synth_out(synth, PROCSPEECH);
 }
 
 module_param_named(ser, synth_apollo.ser, int, S_IRUGO);
Index: linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
@@ -104,6 +104,7 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
+   .io_ops = _serial_io_ops,
.probe = synth_probe,
.release = 

[patch 5/7] staging: speakup: move those functions which do outgoing serial comms, into serialio.c

2017-03-13 Thread okash . khawaja
This moves spk_synth_immediate and spk_serial_synth_probe functions into
serialio.c. These functions do outgoing serial comms. The move is a step
towards collecting all serial comms in serialio.c. This also renames
spk_synth_immediate to spk_serial_synth_immediate.

Code inside those functions has not been changed. Along the way, this patch
also fixes a couple of spots which were calling spk_synth_immediate directly,
so that the calls now happen via the spk_syth struct.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/staging/speakup/serialio.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/serialio.c
+++ linux-4.10.1/drivers/staging/speakup/serialio.c
@@ -136,6 +136,35 @@
outb(1, speakup_info.port_tts + UART_FCR);  /* Turn FIFO On */
 }
 
+int spk_serial_synth_probe(struct spk_synth *synth)
+{
+   const struct old_serial_port *ser;
+   int failed = 0;
+
+   if ((synth->ser >= SPK_LO_TTY) && (synth->ser <= SPK_HI_TTY)) {
+   ser = spk_serial_init(synth->ser);
+   if (ser == NULL) {
+   failed = -1;
+   } else {
+   outb_p(0, ser->port);
+   mdelay(1);
+   outb_p('\r', ser->port);
+   }
+   } else {
+   failed = -1;
+   pr_warn("ttyS%i is an invalid port\n", synth->ser);
+   }
+   if (failed) {
+   pr_info("%s: not found\n", synth->long_name);
+   return -ENODEV;
+   }
+   pr_info("%s: ttyS%i, Driver Version %s\n",
+   synth->long_name, synth->ser, synth->version);
+   synth->alive = 1;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(spk_serial_synth_probe);
+
 void spk_stop_serial_interrupt(void)
 {
if (speakup_info.port_tts == 0)
@@ -223,6 +252,23 @@
return 0;
 }
 
+const char *spk_serial_synth_immediate(struct spk_synth *synth, const char 
*buff)
+{
+   u_char ch;
+
+   while ((ch = *buff)) {
+   if (ch == '\n')
+   ch = synth->procspeech;
+   if (spk_wait_for_xmitr(synth))
+   outb(ch, speakup_info.port_tts);
+   else
+   return buff;
+   buff++;
+   }
+   return NULL;
+}
+EXPORT_SYMBOL_GPL(spk_serial_synth_immediate);
+
 void spk_serial_release(void)
 {
spk_stop_serial_interrupt();
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
@@ -102,7 +102,7 @@
.io_ops = _serial_io_ops,
.probe = synth_probe,
.release = spk_serial_release,
-   .synth_immediate = spk_synth_immediate,
+   .synth_immediate = spk_serial_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
@@ -127,7 +127,7 @@
 
failed = spk_serial_synth_probe(synth);
if (failed == 0) {
-   spk_synth_immediate(synth, "\033=R\r");
+   synth->synth_immediate(synth, "\033=R\r");
mdelay(100);
}
synth->alive = !failed;
Index: linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_apollo.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_apollo.c
@@ -111,7 +111,7 @@
.io_ops = _serial_io_ops,
.probe = spk_serial_synth_probe,
.release = spk_serial_release,
-   .synth_immediate = spk_synth_immediate,
+   .synth_immediate = spk_serial_synth_immediate,
.catch_up = do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_audptr.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_audptr.c
@@ -107,7 +107,7 @@
.io_ops = _serial_io_ops,
.probe = synth_probe,
.release = spk_serial_release,
-   .synth_immediate = spk_synth_immediate,
+   .synth_immediate = spk_serial_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = synth_flush,
.is_alive = spk_synth_is_alive_restart,
@@ -144,7 +144,7 @@
unsigned char test = 0;
char synth_id[40] = "";
 
-   spk_synth_immediate(synth, "\x05[Q]");
+   synth->synth_immediate(synth, "\x05[Q]");
synth_id[test] = spk_serial_in();
if (synth_id[test] == 'A') {
do {
Index: linux-4.10.1/drivers/staging/speakup/speakup_bns.c

[patch 7/7] staging: speakup: migrate acntsa, bns, dummy and txprt to ttyio

2017-03-13 Thread okash . khawaja
This changes the above five synths to TTY-based comms. They were chosen as a
first pass because their serial comms are straightforward, i.e. they don't use
serial input and don't do internal port knocking.

Signed-off-by: Okash Khawaja 

Reviewed-by: Samuel Thibault 

Index: linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_dummy.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_dummy.c
@@ -98,10 +98,10 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
-   .io_ops = _serial_io_ops,
-   .probe = spk_serial_synth_probe,
-   .release = spk_serial_release,
-   .synth_immediate = spk_serial_synth_immediate,
+   .io_ops = _ttyio_ops,
+   .probe = spk_ttyio_synth_probe,
+   .release = spk_ttyio_release,
+   .synth_immediate = spk_ttyio_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
Index: linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_acntsa.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_acntsa.c
@@ -99,10 +99,10 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
-   .io_ops = _serial_io_ops,
+   .io_ops = _ttyio_ops,
.probe = synth_probe,
-   .release = spk_serial_release,
-   .synth_immediate = spk_serial_synth_immediate,
+   .release = spk_ttyio_release,
+   .synth_immediate = spk_ttyio_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,
@@ -125,7 +125,7 @@
 {
int failed;
 
-   failed = spk_serial_synth_probe(synth);
+   failed = spk_ttyio_synth_probe(synth);
if (failed == 0) {
synth->synth_immediate(synth, "\033=R\r");
mdelay(100);
Index: linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
===
--- linux-4.10.1.orig/drivers/staging/speakup/speakup_txprt.c
+++ linux-4.10.1/drivers/staging/speakup/speakup_txprt.c
@@ -95,10 +95,10 @@
.startup = SYNTH_START,
.checkval = SYNTH_CHECK,
.vars = vars,
-   .io_ops = _serial_io_ops,
-   .probe = spk_serial_synth_probe,
-   .release = spk_serial_release,
-   .synth_immediate = spk_serial_synth_immediate,
+   .io_ops = _ttyio_ops,
+   .probe = spk_ttyio_synth_probe,
+   .release = spk_ttyio_release,
+   .synth_immediate = spk_ttyio_synth_immediate,
.catch_up = spk_do_catch_up,
.flush = spk_synth_flush,
.is_alive = spk_synth_is_alive_restart,

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


Re: [PATCH v3 1/2] staging: speakup: identation should use tabs

2017-03-13 Thread Greg KH
On Tue, Mar 14, 2017 at 01:49:54AM +0530, Arushi Singhal wrote:
> Indentation should always use tabs and never spaces.
> 
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/speakup/speakup_dtlk.h | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

What changed from v2?

Always list it, even if nothing changed...

thanks,

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


Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

2017-03-13 Thread Laura Abbott
On 03/13/2017 02:29 PM, Rob Clark wrote:
> On Mon, Mar 13, 2017 at 5:09 PM, Laura Abbott  wrote:
>>> Hm, we might want to expose all the heaps as individual
>>> /dev/ion_$heapname nodes? Should we do this from the start, since
>>> we're massively revamping the uapi anyway (imo not needed, current
>>> state seems to work too)?
>>> -Daniel
>>>
>>
>> I thought about that. One advantage with separate /dev/ion_$heap
>> is that we don't have to worry about a limit of 32 possible
>> heaps per system (32-bit heap id allocation field). But dealing
>> with an ioctl seems easier than names. Userspace might be less
>> likely to hardcode random id numbers vs. names as well.
> 
> 
> other advantage, I think, is selinux (brought up elsewhere on this
> thread).. heaps at known fixed PAs are useful for certain sorts of
> attacks so being able to restrict access more easily seems like a good
> thing
> 
> BR,
> -R
> 

Some other kind of filtering (BPF/LSM/???) might work as well
(http://kernsec.org/files/lss2015/vanderstoep.pdf ?)

The fixed PA issue is a larger problem. We're never going to
be able to get away from "this heap must exist at address X"
problems but the location of CMA in general should be
randomized. I haven't actually come up with a good proposal
to this though.

I'd like for Ion to be a framework for memory allocation and
not security exploits. Hopefully this isn't a pipe dream.

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


Re: [PATCH 1/2] Formatting updates to remove checkpatch warnings in ks_wlan_ioctl.h.

2017-03-13 Thread Greg KH
On Sun, Mar 05, 2017 at 01:52:19PM -0800, Matthew Giassa wrote:
> Implementing some minor formatting changes to remove checkpatch warnings.
> Removing space-hardtab instances. Removing C++-style line comments in favor of
> C-style equivalent. Re-aligning function prototype arguments to remove related
> checkpatch warning.

That is a lot of different things to do all at once, please break this
up into one logical patch per "thing" you do.

Also, fix up your subject line to match how other patches for this
subsystem/driver look, git log will show you what to do there.

thanks,

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


Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event

2017-03-13 Thread Steve Longerbeam



On 03/13/2017 10:10 AM, Hans Verkuil wrote:

On 03/13/2017 06:06 PM, Steve Longerbeam wrote:



On 03/13/2017 03:53 AM, Hans Verkuil wrote:

On 03/13/2017 11:45 AM, Russell King - ARM Linux wrote:

On Mon, Mar 13, 2017 at 11:02:34AM +0100, Hans Verkuil wrote:

On 03/11/2017 07:14 PM, Steve Longerbeam wrote:

The event must be user visible, otherwise the user has no indication
the error, and can't correct it by stream restart.


In that case the driver can detect this and call vb2_queue_error. It's
what it is there for.

The event doesn't help you since only this driver has this issue. So nobody
will watch this event, unless it is sw specifically written for this SoC.

Much better to call vb2_queue_error to signal a fatal error (which this
apparently is) since there are more drivers that do this, and vivid supports
triggering this condition as well.


So today, I can fiddle around with the IMX219 registers to help gain
an understanding of how this sensor works.  Several of the registers
(such as the PLL setup [*]) require me to disable streaming on the
sensor while changing them.

This is something I've done many times while testing various ideas,
and is my primary way of figuring out and testing such things.

Whenever I resume streaming (provided I've let the sensor stop
streaming at a frame boundary) it resumes as if nothing happened.  If I
stop the sensor mid-frame, then I get the rolling issue that Steve
reports, but once the top of the frame becomes aligned with the top of
the capture, everything then becomes stable again as if nothing happened.

The side effect of what you're proposing is that when I disable streaming
at the sensor by poking at its registers, rather than the capture just
stopping, an error is going to be delivered to gstreamer, and gstreamer
is going to exit, taking the entire capture process down.

This severely restricts the ability to be able to develop and test
sensor drivers.

So, I strongly disagree with you.

Loss of capture frames is not necessarily a fatal error - as I have been
saying repeatedly.  In Steve's case, there's some unknown interaction
between the source and iMX6 hardware that is causing the instability,
but that is simply not true of other sources, and I oppose any idea that
we should cripple the iMX6 side of the capture based upon just one
hardware combination where this is a problem.

Steve suggested that the problem could be in the iMX6 CSI block - and I
note comparing Steve's code with the code in FSL's repository that there
are some changes that are missing in Steve's code to do with the CCIR656
sync code setup, particularly for >8 bit.  The progressive CCIR656 8-bit
setup looks pretty similar though - but I think what needs to be asked
is whether the same problem is visible using the FSL/NXP vendor kernel.


* - the PLL setup is something that requires research at the moment.
Sony's official position (even to their customers) is that they do not
supply the necessary information, instead they expect customers to tell
them the capture settings they want, and Sony will throw the values into
a spreadsheet, and they'll supply the register settings back to the
customer.  Hence, the only way to proceed with a generic driver for
this sensor is to experiment, and experimenting requires the ability to
pause the stream at the sensor while making changes.  Take this away,
and we're stuck with the tables-of-register-settings-for-set-of-fixed-
capture-settings approach.  I've made a lot of progress away from this
which is all down to the flexibility afforded by _not_ killing the
capture process.



In other words: Steve should either find a proper fix for this, or only
call vb2_queue_error in this specific case. Sending an event that nobody
will know how to handle or what to do with is pretty pointless IMHO.

Let's just give him time to try and figure out the real issue here.



This is a long-standing issue, I've traveled to Hildesheim working with
our customer to try and get to the bottom of it. I can go into a lot of
details from those trips, we probed the bt.656 bus with a logic analyzer
and I can share those results with anyone who asks. But the results of
those investigations indicate the CSI is not handling the SAV/EAV sync
codes correctly - if there is a shift in the line position at which
those codes occur, the CSI/IPU does not abort the frame capture DMA
and start from the new sync code position, it just continues to capture
lines until the programmed number of lines are transferred, hence you
get these split images. Freescale also informed us of a mechanism in the
IPU that will add lines if it detects these short frames, until the
programmed number of lines are reached. Apparently that is what creates
the rolling effect, but this rolling can last for up to a full minute,
which is completely unacceptable, it must be corrected as soon as
possible.

So the only thing we could come up with was to monitor frame intervals,
this is purely empirical, but we 

Re: [Outreachy kernel] [PATCH v3 4/4] staging: sm750fb: Remove typedefs

2017-03-13 Thread Julia Lawall


On Tue, 14 Mar 2017, Arushi Singhal wrote:

> This patch removes typedefs from structures and renames them as per
> kernel coding standards.

Greg requested that typedef removing patches adjust only one patch at a
time.

The names of the types typically need to be adjusted as compared to what
is in the typedef.  In your cases, the type names begin with _ and end
with _t.  Beginning with _ means that something is sort of internal,
rarely used etc.  That made sense when there was a typedef, but since you
are removing that, the _ at the front should go too.  Likewise, _t is an
extension that indicates a typedef, but you don't have that any more, so
the _t should be dropped too.

Finally, the log message talks about structures, but the changes involve
one structure and one enum.  The log message should describe everything
that has changed.

julia

>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/sm750fb/ddk750_mode.c |  6 +++---
>  drivers/staging/sm750fb/ddk750_mode.h | 20 +---
>  drivers/staging/sm750fb/sm750_hw.c|  2 +-
>  3 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
> b/drivers/staging/sm750fb/ddk750_mode.c
> index eea5aef2956f..25da678179f7 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -12,7 +12,7 @@
>   * HW only supports 7 predefined pixel clocks, and clock select is
>   * in bit 29:27 of Display Control register.
>   */
> -static unsigned long displayControlAdjust_SM750LE(mode_parameter_t 
> *pModeParam, unsigned long dispControl)
> +static unsigned long displayControlAdjust_SM750LE(struct _mode_parameter_t 
> *pModeParam, unsigned long dispControl)
>  {
>   unsigned long x, y;
>
> @@ -72,7 +72,7 @@ static unsigned long 
> displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
>  }
>
>  /* only timing related registers will be  programed */
> -static int programModeRegisters(mode_parameter_t *pModeParam, struct 
> pll_value *pll)
> +static int programModeRegisters(struct _mode_parameter_t *pModeParam, struct 
> pll_value *pll)
>  {
>   int ret = 0;
>   int cnt = 0;
> @@ -198,7 +198,7 @@ static int programModeRegisters(mode_parameter_t 
> *pModeParam, struct pll_value *
>   return ret;
>  }
>
> -int ddk750_setModeTiming(mode_parameter_t *parm, clock_type_t clock)
> +int ddk750_setModeTiming(struct _mode_parameter_t *parm, clock_type_t clock)
>  {
>   struct pll_value pll;
>   unsigned int uiActualPixelClk;
> diff --git a/drivers/staging/sm750fb/ddk750_mode.h 
> b/drivers/staging/sm750fb/ddk750_mode.h
> index 6d204b8b4a01..5f8347d87800 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.h
> +++ b/drivers/staging/sm750fb/ddk750_mode.h
> @@ -3,37 +3,35 @@
>
>  #include "ddk750_chip.h"
>
> -typedef enum _spolarity_t {
> +enum _spolarity_t {
>   POS = 0, /* positive */
>   NEG, /* negative */
> -}
> -spolarity_t;
> +};
>
> -typedef struct _mode_parameter_t {
> +struct _mode_parameter_t {
>   /* Horizontal timing. */
>   unsigned long horizontal_total;
>   unsigned long horizontal_display_end;
>   unsigned long horizontal_sync_start;
>   unsigned long horizontal_sync_width;
> - spolarity_t horizontal_sync_polarity;
> + enum _spolarity_t horizontal_sync_polarity;
>
>   /* Vertical timing. */
>   unsigned long vertical_total;
>   unsigned long vertical_display_end;
>   unsigned long vertical_sync_start;
>   unsigned long vertical_sync_height;
> - spolarity_t vertical_sync_polarity;
> + enum _spolarity_t vertical_sync_polarity;
>
>   /* Refresh timing. */
>   unsigned long pixel_clock;
>   unsigned long horizontal_frequency;
> - unsigned long vertical_frequency;
> + enum _spolarity_t vertical_frequency;
>
>   /* Clock Phase. This clock phase only applies to Panel. */
> - spolarity_t clock_phase_polarity;
> -}
> -mode_parameter_t;
> + enum _spolarity_t clock_phase_polarity;
> +};
>
> -int ddk750_setModeTiming(mode_parameter_t *parm, clock_type_t clock);
> +int ddk750_setModeTiming(struct _mode_parameter_t *parm, clock_type_t clock);
>
>  #endif
> diff --git a/drivers/staging/sm750fb/sm750_hw.c 
> b/drivers/staging/sm750fb/sm750_hw.c
> index fab3fc9c8330..4233e0bf12a7 100644
> --- a/drivers/staging/sm750fb/sm750_hw.c
> +++ b/drivers/staging/sm750fb/sm750_hw.c
> @@ -252,7 +252,7 @@ int hw_sm750_crtc_setMode(struct lynxfb_crtc *crtc,
>  {
>   int ret, fmt;
>   u32 reg;
> - mode_parameter_t modparm;
> + struct _mode_parameter_t modparm;
>   clock_type_t clock;
>   struct sm750_dev *sm750_dev;
>   struct lynxfb_par *par;
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

2017-03-13 Thread Laura Abbott
On 03/13/2017 06:21 AM, Mark Brown wrote:
> On Mon, Mar 13, 2017 at 10:54:33AM +, Brian Starkey wrote:
>> On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote:
> 
>>> Another point is how can we put secure rules (like selinux policy) on
>>> heaps since all the allocations
>>> go to the same device (/dev/ion) ? For example, until now, in Android
>>> we have to give the same
>>> access rights to all the process that use ION.
>>> It will become problem when we will add secure heaps because we won't
>>> be able to distinguish secure
>>> processes to standard ones or set specific policy per heaps.
>>> Maybe I'm wrong here but I have never see selinux policy checking an
>>> ioctl field but if that
>>> exist it could be a solution.
> 
>> I might be thinking of a different type of "secure", but...
> 
>> Should the security of secure heaps be enforced by OS-level
>> permissions? I don't know about other architectures, but at least on
>> arm/arm64 this is enforced in hardware; it doesn't matter who has
>> access to the ion heap, because only secure devices (or the CPU
>> running a secure process) is physically able to access the memory
>> backing the buffer.
> 3
>> In fact, in the use-cases I know of, the process asking for the ion
>> allocation is not a secure process, and so we wouldn't *want* to
>> restrict the secure heap to be allocated from only by secure
>> processes.
> 
> I think there's an orthogonal level of OS level security that can be
> applied here - it's reasonable for it to want to say things like "only
> processes that are supposed to be implementing functionality X should be
> able to try to allocate memory set aside for that functionality".  This
> mitigates against escallation attacks and so on, it's not really
> directly related to secure memory as such though.
> 

Ion also makes it pretty trivial to allocate large amounts of kernel
memory and possibly DoS the system. I'd like to have as little
policy in Ion as possible but more important would be a general
security review and people shouting "bad idea ahead".

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


Re: [PATCH v2 1/2] staging: speakup: Added blank line after function/struct/union/enum declarations

2017-03-13 Thread Daniele Nicolodi
Hello,

On 3/13/17 2:40 PM, Arushi Singhal wrote:
> This patch fixes the warnings reported by checkpatch.pl
> for please use a blank line after function/struct/union/enum
> declarations.

I haven't seen this pointed out by others before: starting a commit
message with "This patch..." is redundant (it is a commit message, thus
what it describes is surely a patch) and most times leads to bad style
commit messages.

In this case the commit message can simply be:

Add a blank line after enum and struct declarations.

or

Ensure that enum and struct declarations are followed by a blank line.

Cheers,
Daniele


> 
> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - change the subject to make it more relevant.
> 
>  drivers/staging/speakup/main.c | 1 +
>  drivers/staging/speakup/serialio.c | 1 +
>  drivers/staging/speakup/speakup_dtlk.c | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index 6786cfab7460..4b750ec8cd59 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -108,6 +108,7 @@ enum {
>   CT_Window,
>   CT_Max
>  };
> +
>  #define read_all_mode CT_Max
>  
>  static struct tty_struct *tty;
> diff --git a/drivers/staging/speakup/serialio.c 
> b/drivers/staging/speakup/serialio.c
> index aade52ee15a0..d51e382cb835 100644
> --- a/drivers/staging/speakup/serialio.c
> +++ b/drivers/staging/speakup/serialio.c
> @@ -21,6 +21,7 @@ static void start_serial_interrupt(int irq);
>  static const struct old_serial_port rs_table[] = {
>   SERIAL_PORT_DFNS
>  };
> +
>  static const struct old_serial_port *serstate;
>  static int timeouts;
>  
> diff --git a/drivers/staging/speakup/speakup_dtlk.c 
> b/drivers/staging/speakup/speakup_dtlk.c
> index eede2d82183a..c0b2208aa8a7 100644
> --- a/drivers/staging/speakup/speakup_dtlk.c
> +++ b/drivers/staging/speakup/speakup_dtlk.c
> @@ -43,6 +43,7 @@ static int port_forced;
>  static unsigned int synth_portlist[] = {
>0x25e, 0x29e, 0x2de, 0x31e, 0x35e, 0x39e, 0
>  };
> +
>  static u_char synth_status;
>  
>  static struct var_t vars[] = {
> 

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


[PATCH] Staging: andorid: ion: ion_dummy_driver: remove unnecessary empty line

2017-03-13 Thread Maciej Billewicz
From: Maciej Billewicz 

Fix coding style issue.

Signed-off-by: Maciej Billewicz 
---
 drivers/staging/android/ion/ion_dummy_driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_dummy_driver.c 
b/drivers/staging/android/ion/ion_dummy_driver.c
index cf5c010..c1f9b83 100644
--- a/drivers/staging/android/ion/ion_dummy_driver.c
+++ b/drivers/staging/android/ion/ion_dummy_driver.c
@@ -75,7 +75,6 @@ static int __init ion_dummy_init(void)
if (!heaps)
return -ENOMEM;
 
-
/* Allocate a dummy carveout heap */
carveout_ptr = alloc_pages_exact(
dummy_heaps[ION_HEAP_TYPE_CARVEOUT].size,
-- 
2.7.4

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


Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

2017-03-13 Thread Russell King - ARM Linux
On Mon, Mar 13, 2017 at 11:03:50PM +0200, Sakari Ailus wrote:
> Hi Steve,
> 
> On Mon, Mar 13, 2017 at 11:06:22AM -0700, Steve Longerbeam wrote:
> > I'm kinda in the middle on this topic. I agree with Sakari that
> > frame rate can fluctuate, but that should only be temporary. If
> > the frame rate permanently shifts from what a subdev reports via
> > g_frame_interval, then that is a system problem. So I agree with
> > Phillip and Russell that a link validation of frame interval still
> > makes sense.
> > 
> > But I also have to agree with Sakari that a subdev that has no
> > control over frame rate has no business implementing those ops.
> > 
> > And then I agree with Russell that for subdevs that do have control
> > over frame rate, they would have to walk the graph to find the frame
> > rate source.
> > 
> > So we're stuck in a broken situation: either the subdevs have to walk
> > the graph to find the source of frame rate, or s_frame_interval
> > would have to be mandatory and validated between pads, same as set_fmt.
> 
> It's not broken; what we are missing though is documentation on how to
> control devices that can change the frame rate i.e. presumably drop frames
> occasionally.

It's not about "presumably drop frames occasionally."  The definition
of "occasional" is "occurring or appearing at irregular or infrequent
intervals".  Another word which describes what you're saying would be
"randomly drop frames" which would be a quite absurd "feature" to
include in hardware.

It's about _deterministically_ omitting frames from the capture.

The hardware provides two controls:
1. the size of a group of frames - between 1 and 5 frames
2. select which frames within the group are dropped using a bitmask

This gives a _regular_ pattern of dropped frames.

The rate scaling is given by: hweight(bitmask) / group size.

So, for example, if you're receiving a 50fps TV broadcast, and want to
capture at 25fps, you can set the group size to 2, and set the frame
drop to binary "01" or "10" - if it's interlaced, this would have the
effect of selecting one field, or skipping every other frame if
progressive.

That's not "we'll occasionally drop some frames", that's frame rate
scaling.  Just like you can scale the size of an image by omitting
every other pixel and every other line, this hardware allows omitting
every other frame or more.

So, to configure this feature, CSI needs to know two bits of information:

1. The _source_ frame rate.
2. The desired _sink_ frame rate.

>From that, it can compute how many frames to drop, and size of the group.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

2017-03-13 Thread Rob Clark
On Mon, Mar 13, 2017 at 5:09 PM, Laura Abbott  wrote:
>> Hm, we might want to expose all the heaps as individual
>> /dev/ion_$heapname nodes? Should we do this from the start, since
>> we're massively revamping the uapi anyway (imo not needed, current
>> state seems to work too)?
>> -Daniel
>>
>
> I thought about that. One advantage with separate /dev/ion_$heap
> is that we don't have to worry about a limit of 32 possible
> heaps per system (32-bit heap id allocation field). But dealing
> with an ioctl seems easier than names. Userspace might be less
> likely to hardcode random id numbers vs. names as well.


other advantage, I think, is selinux (brought up elsewhere on this
thread).. heaps at known fixed PAs are useful for certain sorts of
attacks so being able to restrict access more easily seems like a good
thing

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


Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

2017-03-13 Thread Laura Abbott
On 03/13/2017 03:54 AM, Brian Starkey wrote:
> On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote:
>> 2017-03-09 18:38 GMT+01:00 Laura Abbott :
>>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>
>>> No one gave a thing about android in upstream, so Greg KH just dumped it
>>> all into staging/android/. We've discussed ION a bunch of times, 
>>> recorded
>>> anything we'd like to fix in staging/android/TODO, and Laura's patch
>>> series here addresses a big chunk of that.
>>
>>> This is pretty much the same approach we (gpu folks) used to de-stage 
>>> the
>>> syncpt stuff.
>>
>> Well, there's also the fact that quite a few people have issues with the
>> design (like Laurent).  It seems like a lot of them have either got more
>> comfortable with it over time, or at least not managed to come up with
>> any better ideas in the meantime.
>
> See the TODO, it has everything a really big group (look at the patch for
> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
> merge stuff because merging stuff is fun :-)
>
> Laurent was even in that group ...
> -Daniel

 For me those patches are going in the right direction.

 I still have few questions:
 - since alignment management has been remove from ion-core, should it
 be also removed from ioctl structure ?
>>>
>>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>>> so we don't need the compat layer and as part of that I'm also
>>> dropping the align argument.
>>>
 - can you we ride off ion_handle (at least in userland) and only
 export a dma-buf descriptor ?
>>>
>>> Yes, I think this is the right direction given we're breaking
>>> everything anyway. I was debating trying to keep the two but
>>> moving to only dma bufs is probably cleaner. The only reason
>>> I could see for keeping the handles is running out of file
>>> descriptors for dma-bufs but that seems unlikely.

 In the future how can we add new heaps ?
 Some platforms have very specific memory allocation
 requirements (just have a look in the number of gem custom allocator in 
 drm)
 Do you plan to add heap type/mask for each ?
>>>
>>> Yes, that was my thinking.
>>
>> My concern is about the policy to adding heaps, will you accept
>> "customs" heap per
>> platforms ? per devices ? or only generic ones ?
>> If you are too strict, we will have lot of out-of-tree heaps and if
>> you accept of of them
>> it will be a nightmare to maintain
>>
> 
> Are you concerned about actual heaps (e.g. a carveout at 0x8000 vs
> a carveout at 0x6000) or heap types?
> 
> For heap types, I think the policy can be strict - if it's generally
> useful then it should live in-tree in ion. Otherwise, it would be
> out-of-tree. I'd expect most "custom" heaps to be parameterisable to
> the point of being generally useful.
> 

I'm willing to be reasonably permissive in what lives in tree. A good
example would be something like a heap for the OMAP tiler which had
weird hardware requirements. The associated devices that go with the
heap should be well supported upstream though.

> For actual heap instances, I would expect them to be communicated via
> reserved-memory regions or something similar, and so the maintenance
> burden is pretty low.
> 

Yes. After the next round of review for this series I'm going to
start thinking about properties for chunk and carveout heaps if nobody
proposes something first.

> The existing query ioctl can allow heap IDs to get assigned
> dynamically at runtime, so there's no need to reserve "bit 6" for
> "CUSTOM_ACME_HEAP_1"
> 
>> Another point is how can we put secure rules (like selinux policy) on
>> heaps since all the allocations
>> go to the same device (/dev/ion) ? For example, until now, in Android
>> we have to give the same
>> access rights to all the process that use ION.
>> It will become problem when we will add secure heaps because we won't
>> be able to distinguish secure
>> processes to standard ones or set specific policy per heaps.
>> Maybe I'm wrong here but I have never see selinux policy checking an
>> ioctl field but if that
>> exist it could be a solution.
>>
> 
> I might be thinking of a different type of "secure", but...
> 
> Should the security of secure heaps be enforced by OS-level
> permissions? I don't know about other architectures, but at least on
> arm/arm64 this is enforced in hardware; it doesn't matter who has
> access to the ion heap, because only secure devices (or the CPU
> running a secure process) is physically able to access the memory
> backing the buffer.
> 
> In fact, in the use-cases I know of, the process asking for the ion
> 

[PATCH] Staging: andorid: ion: ion_dummy_driver: remove unnecessary empty line

2017-03-13 Thread Maciej Billewicz
Fix coding style issue.

Signed-off-by: Maciej Billewicz 
---
 drivers/staging/android/ion/ion_dummy_driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_dummy_driver.c 
b/drivers/staging/android/ion/ion_dummy_driver.c
index cf5c010..c1f9b83 100644
--- a/drivers/staging/android/ion/ion_dummy_driver.c
+++ b/drivers/staging/android/ion/ion_dummy_driver.c
@@ -75,7 +75,6 @@ static int __init ion_dummy_init(void)
if (!heaps)
return -ENOMEM;
 
-
/* Allocate a dummy carveout heap */
carveout_ptr = alloc_pages_exact(
dummy_heaps[ION_HEAP_TYPE_CARVEOUT].size,
-- 
2.7.4

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


Re: [Outreachy kernel] [PATCH v2 1/2] staging: speakup: Added blank line after function/struct/union/enum declarations

2017-03-13 Thread Julia Lawall
Remember to use the imperative.  "Add blank line...".  You could just drop
function/struct/union/enum.  Just declarations is fine.

On Tue, 14 Mar 2017, Arushi Singhal wrote:

> This patch fixes the warnings reported by checkpatch.pl
> for please use a blank line after function/struct/union/enum
> declarations.

Again, it does not say what you have done, and only focuses on the message
you have fixed.

julia

> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - change the subject to make it more relevant.
>
>  drivers/staging/speakup/main.c | 1 +
>  drivers/staging/speakup/serialio.c | 1 +
>  drivers/staging/speakup/speakup_dtlk.c | 1 +
>  3 files changed, 3 insertions(+)
>
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index 6786cfab7460..4b750ec8cd59 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -108,6 +108,7 @@ enum {
>   CT_Window,
>   CT_Max
>  };
> +
>  #define read_all_mode CT_Max
>
>  static struct tty_struct *tty;
> diff --git a/drivers/staging/speakup/serialio.c 
> b/drivers/staging/speakup/serialio.c
> index aade52ee15a0..d51e382cb835 100644
> --- a/drivers/staging/speakup/serialio.c
> +++ b/drivers/staging/speakup/serialio.c
> @@ -21,6 +21,7 @@ static void start_serial_interrupt(int irq);
>  static const struct old_serial_port rs_table[] = {
>   SERIAL_PORT_DFNS
>  };
> +
>  static const struct old_serial_port *serstate;
>  static int timeouts;
>
> diff --git a/drivers/staging/speakup/speakup_dtlk.c 
> b/drivers/staging/speakup/speakup_dtlk.c
> index eede2d82183a..c0b2208aa8a7 100644
> --- a/drivers/staging/speakup/speakup_dtlk.c
> +++ b/drivers/staging/speakup/speakup_dtlk.c
> @@ -43,6 +43,7 @@ static int port_forced;
>  static unsigned int synth_portlist[] = {
>0x25e, 0x29e, 0x2de, 0x31e, 0x35e, 0x39e, 0
>  };
> +
>  static u_char synth_status;
>
>  static struct var_t vars[] = {
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170313204018.24601-2-arushisinghal19971997%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH v2 2/2] staging: speakup: fix "Alignment match open parenthesis"

2017-03-13 Thread Julia Lawall


On Tue, 14 Mar 2017, Arushi Singhal wrote:

> Fix checkpatch issues: "CHECK: Alignment should match open parenthesis".

You don't have to send another revision just for this issue, but the
commit message would be better as "Align arguments with open parenthesis".
Something that explains what you did, rather than what warning message you
fixed.  In this case, the thing you did is pretty obvious from the warning
message, but that might not always be the case.  So it is better to get
used to writing commit messages that explain what you did and why, rather
than what error message you fixed.  If you force yourself to write a
commit message without using the word fix, you will probably come up with
a good solution.

julia

>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/speakup/kobjects.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/speakup/kobjects.c 
> b/drivers/staging/speakup/kobjects.c
> index 8a586323b728..ca85476e3ff7 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -93,7 +93,7 @@ static void report_char_chartab_status(int reset, int 
> received, int used,
>   } else if (received) {
>   len = snprintf(buf, sizeof(buf),
>  " updated %d of %d %s\n",
> - used, received, object_type[do_characters]);
> +used, received, object_type[do_characters]);
>   if (rejected)
>   snprintf(buf + (len - 1), sizeof(buf) - (len - 1),
>" with %d reject%s\n",
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170313204018.24601-3-arushisinghal19971997%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

2017-03-13 Thread Russell King - ARM Linux
On Mon, Mar 13, 2017 at 10:56:46PM +0200, Sakari Ailus wrote:
> Hi Russell,
> 
> On Mon, Mar 13, 2017 at 01:27:02PM +, Russell King - ARM Linux wrote:
> > On Mon, Mar 13, 2017 at 03:16:48PM +0200, Sakari Ailus wrote:
> > > The vast majority of existing drivers do not implement them nor the user
> > > space expects having to set them. Making that mandatory would break 
> > > existing
> > > user space.
> > > 
> > > In addition, that does not belong to link validation either: link 
> > > validation
> > > should only include static properties of the link that are required for
> > > correct hardware operation. Frame rate is not such property: hardware that
> > > supports the MC interface generally does not recognise such concept (with
> > > the exception of some sensors). Additionally, it is dynamic: the frame 
> > > rate
> > > can change during streaming, making its validation at streamon time 
> > > useless.
> > 
> > So how do we configure the CSI, which can do frame skipping?
> > 
> > With what you're proposing, it means it's possible to configure the
> > camera sensor source pad to do 50fps.  Configure the CSI sink pad to
> > an arbitary value, such as 30fps, and configure the CSI source pad to
> > 15fps.
> > 
> > What you actually get out of the CSI is 25fps, which bears very little
> > with the actual values used on the CSI source pad.
> > 
> > You could say "CSI should ask the camera sensor" - well, that's fine
> > if it's immediately downstream, but otherwise we'd need to go walking
> > down the graph to find something that resembles its source - there may
> > be mux and CSI2 interface subdev blocks in that path.  Or we just accept
> > that frame rates are completely arbitary and bear no useful meaning what
> > so ever.
> 
> The user is responsible for configuring the pipeline. It is thus not
> unreasonable to as the user to configure the frame rate as well if there are
> device in the pipeline that can affect the frame rate. The way I proposed to
> implement it is compliant with the existing API and entirely deterministic,
> contrary to what you're saying.

You haven't really addressed my point at all.

What you seem to be saying is that you're quite happy for the situation
(which is a total misconfiguration) to exist.  Given the vapourware of
userspace (which I don't see changing in any kind of reasonable timeline)
I think this is completely absurd.

I'll state clearly now: everything that we've discussed so far, I'm
finding very hard to take anything you've said seriously.  I think we
have very different and incompatible point of views about what is
acceptable from a user point of view, so much so that we're never going
to agree on any point.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

2017-03-13 Thread Laura Abbott
On 03/12/2017 12:05 PM, Daniel Vetter wrote:
> On Sun, Mar 12, 2017 at 2:34 PM, Benjamin Gaignard
>  wrote:
>> 2017-03-09 18:38 GMT+01:00 Laura Abbott :
>>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>
>>> No one gave a thing about android in upstream, so Greg KH just dumped it
>>> all into staging/android/. We've discussed ION a bunch of times, 
>>> recorded
>>> anything we'd like to fix in staging/android/TODO, and Laura's patch
>>> series here addresses a big chunk of that.
>>
>>> This is pretty much the same approach we (gpu folks) used to de-stage 
>>> the
>>> syncpt stuff.
>>
>> Well, there's also the fact that quite a few people have issues with the
>> design (like Laurent).  It seems like a lot of them have either got more
>> comfortable with it over time, or at least not managed to come up with
>> any better ideas in the meantime.
>
> See the TODO, it has everything a really big group (look at the patch for
> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
> merge stuff because merging stuff is fun :-)
>
> Laurent was even in that group ...
> -Daniel

 For me those patches are going in the right direction.

 I still have few questions:
 - since alignment management has been remove from ion-core, should it
 be also removed from ioctl structure ?
>>>
>>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>>> so we don't need the compat layer and as part of that I'm also
>>> dropping the align argument.
>>>
 - can you we ride off ion_handle (at least in userland) and only
 export a dma-buf descriptor ?
>>>
>>> Yes, I think this is the right direction given we're breaking
>>> everything anyway. I was debating trying to keep the two but
>>> moving to only dma bufs is probably cleaner. The only reason
>>> I could see for keeping the handles is running out of file
>>> descriptors for dma-bufs but that seems unlikely.

 In the future how can we add new heaps ?
 Some platforms have very specific memory allocation
 requirements (just have a look in the number of gem custom allocator in 
 drm)
 Do you plan to add heap type/mask for each ?
>>>
>>> Yes, that was my thinking.
>>
>> My concern is about the policy to adding heaps, will you accept
>> "customs" heap per
>> platforms ? per devices ? or only generic ones ?
>> If you are too strict, we will have lot of out-of-tree heaps and if
>> you accept of of them
>> it will be a nightmare to maintain
> 
> I think ion should expose any heap that's also directly accessible to
> devices using dma_alloc(_coherent). That should leave very few things
> left, like your SMA heap.
> 
>> Another point is how can we put secure rules (like selinux policy) on
>> heaps since all the allocations
>> go to the same device (/dev/ion) ? For example, until now, in Android
>> we have to give the same
>> access rights to all the process that use ION.
>> It will become problem when we will add secure heaps because we won't
>> be able to distinguish secure
>> processes to standard ones or set specific policy per heaps.
>> Maybe I'm wrong here but I have never see selinux policy checking an
>> ioctl field but if that
>> exist it could be a solution.
> 
> Hm, we might want to expose all the heaps as individual
> /dev/ion_$heapname nodes? Should we do this from the start, since
> we're massively revamping the uapi anyway (imo not needed, current
> state seems to work too)?
> -Daniel
> 

I thought about that. One advantage with separate /dev/ion_$heap
is that we don't have to worry about a limit of 32 possible
heaps per system (32-bit heap id allocation field). But dealing
with an ioctl seems easier than names. Userspace might be less
likely to hardcode random id numbers vs. names as well.

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


Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

2017-03-13 Thread Sakari Ailus
Hi Steve,

On Mon, Mar 13, 2017 at 11:06:22AM -0700, Steve Longerbeam wrote:
> 
> 
> On 03/13/2017 06:55 AM, Philipp Zabel wrote:
> >On Mon, 2017-03-13 at 13:27 +, Russell King - ARM Linux wrote:
> >>On Mon, Mar 13, 2017 at 03:16:48PM +0200, Sakari Ailus wrote:
> >>>The vast majority of existing drivers do not implement them nor the user
> >>>space expects having to set them. Making that mandatory would break 
> >>>existing
> >>>user space.
> >>>
> >>>In addition, that does not belong to link validation either: link 
> >>>validation
> >>>should only include static properties of the link that are required for
> >>>correct hardware operation. Frame rate is not such property: hardware that
> >>>supports the MC interface generally does not recognise such concept (with
> >>>the exception of some sensors). Additionally, it is dynamic: the frame rate
> >>>can change during streaming, making its validation at streamon time 
> >>>useless.
> >>
> >>So how do we configure the CSI, which can do frame skipping?
> >>
> >>With what you're proposing, it means it's possible to configure the
> >>camera sensor source pad to do 50fps.  Configure the CSI sink pad to
> >>an arbitary value, such as 30fps, and configure the CSI source pad to
> >>15fps.
> >>
> >>What you actually get out of the CSI is 25fps, which bears very little
> >>with the actual values used on the CSI source pad.
> >>
> >>You could say "CSI should ask the camera sensor" - well, that's fine
> >>if it's immediately downstream, but otherwise we'd need to go walking
> >>down the graph to find something that resembles its source - there may
> >>be mux and CSI2 interface subdev blocks in that path.  Or we just accept
> >>that frame rates are completely arbitary and bear no useful meaning what
> >>so ever.
> >
> >Which would include the frame interval returned by VIDIOC_G_PARM on the
> >connected video device, as that gets its information from the CSI output
> >pad's frame interval.
> >
> 
> I'm kinda in the middle on this topic. I agree with Sakari that
> frame rate can fluctuate, but that should only be temporary. If
> the frame rate permanently shifts from what a subdev reports via
> g_frame_interval, then that is a system problem. So I agree with
> Phillip and Russell that a link validation of frame interval still
> makes sense.
> 
> But I also have to agree with Sakari that a subdev that has no
> control over frame rate has no business implementing those ops.
> 
> And then I agree with Russell that for subdevs that do have control
> over frame rate, they would have to walk the graph to find the frame
> rate source.
> 
> So we're stuck in a broken situation: either the subdevs have to walk
> the graph to find the source of frame rate, or s_frame_interval
> would have to be mandatory and validated between pads, same as set_fmt.

It's not broken; what we are missing though is documentation on how to
control devices that can change the frame rate i.e. presumably drop frames
occasionally.

If you're doing something that hasn't been done before, it may be that new
documentation needs to be written to accomodate that use case. As we have an
existing interface (VIDIOC_SUBDEV_[GS]_FRAME_INTERVAL) it does make sense
to use that. What is not possible, though, is to mandate its use in link
validation everywhere.

If you had a hardware limitation that would require that the frame rate is
constant, then we'd need to handle that in link validation for that
particular piece of hardware. But there really is no case for doing that for
everything else.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: android: ion: Replace pr_err with dev_err

2017-03-13 Thread Laura Abbott
On 03/13/2017 06:56 AM, simran singhal wrote:
> All devm functions has a device structure as the first argument which is
> required by dev_{err,info,dbg} printing functions.
> This patch converts pr_err to dev_err as dev_* is preferred after calls
> to devm functions.
> 
> Done using coccinelle:
> 
> @r1 exists@
> expression e,e1;
> identifier f =~ "^devm_";
> identifier g =~ "^pcim_";
> identifier h =~ "^dmam_";
> @@
> e=\(f\|g\|h\)(e1,...);
> <+...
> (
> - pr_info(
> + dev_info(e1,
>...);
> |
> - pr_err(
> + dev_err(e1,
>   ...);
> |
> - pr_debug(
> + dev_dbg(e1,
>   ...);
> )
> ...+>
> 
> Signed-off-by: simran singhal 
> ---
>  drivers/staging/android/ion/ion_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion_test.c 
> b/drivers/staging/android/ion/ion_test.c
> index 5abf8320..0ab7d11 100644
> --- a/drivers/staging/android/ion/ion_test.c
> +++ b/drivers/staging/android/ion/ion_test.c
> @@ -255,7 +255,7 @@ static int __init ion_test_probe(struct platform_device 
> *pdev)
>   testdev->misc.parent = >dev;
>   ret = misc_register(>misc);
>   if (ret) {
> - pr_err("failed to register misc device.\n");
> + dev_err(>dev, "failed to register misc device.\n");
>   return ret;
>   }
>  
> 

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


Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

2017-03-13 Thread Sakari Ailus
Hi Russell,

On Mon, Mar 13, 2017 at 01:27:02PM +, Russell King - ARM Linux wrote:
> On Mon, Mar 13, 2017 at 03:16:48PM +0200, Sakari Ailus wrote:
> > The vast majority of existing drivers do not implement them nor the user
> > space expects having to set them. Making that mandatory would break existing
> > user space.
> > 
> > In addition, that does not belong to link validation either: link validation
> > should only include static properties of the link that are required for
> > correct hardware operation. Frame rate is not such property: hardware that
> > supports the MC interface generally does not recognise such concept (with
> > the exception of some sensors). Additionally, it is dynamic: the frame rate
> > can change during streaming, making its validation at streamon time useless.
> 
> So how do we configure the CSI, which can do frame skipping?
> 
> With what you're proposing, it means it's possible to configure the
> camera sensor source pad to do 50fps.  Configure the CSI sink pad to
> an arbitary value, such as 30fps, and configure the CSI source pad to
> 15fps.
> 
> What you actually get out of the CSI is 25fps, which bears very little
> with the actual values used on the CSI source pad.
> 
> You could say "CSI should ask the camera sensor" - well, that's fine
> if it's immediately downstream, but otherwise we'd need to go walking
> down the graph to find something that resembles its source - there may
> be mux and CSI2 interface subdev blocks in that path.  Or we just accept
> that frame rates are completely arbitary and bear no useful meaning what
> so ever.

The user is responsible for configuring the pipeline. It is thus not
unreasonable to as the user to configure the frame rate as well if there are
device in the pipeline that can affect the frame rate. The way I proposed to
implement it is compliant with the existing API and entirely deterministic,
contrary to what you're saying.

It's not the job of the CSI sub-device to figure it out.

S_PARM and G_PARM function as interface on the V4L2 API to access the frame
rate on plain V4L2 devices. The i.MX6 IPU is not a plain V4L2 device.
Essentially the V4L2 device node you have there is an interface to a rather
plain DMA engine, no more.

There have been plans to add device profiles to the documentation but that
work has not yet been done.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v3 4/4] staging: sm750fb: Remove typedefs

2017-03-13 Thread Arushi Singhal
This patch removes typedefs from structures and renames them as per
kernel coding standards.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/sm750fb/ddk750_mode.c |  6 +++---
 drivers/staging/sm750fb/ddk750_mode.h | 20 +---
 drivers/staging/sm750fb/sm750_hw.c|  2 +-
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index eea5aef2956f..25da678179f7 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -12,7 +12,7 @@
  * HW only supports 7 predefined pixel clocks, and clock select is
  * in bit 29:27 of Display Control register.
  */
-static unsigned long displayControlAdjust_SM750LE(mode_parameter_t 
*pModeParam, unsigned long dispControl)
+static unsigned long displayControlAdjust_SM750LE(struct _mode_parameter_t 
*pModeParam, unsigned long dispControl)
 {
unsigned long x, y;
 
@@ -72,7 +72,7 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 }
 
 /* only timing related registers will be  programed */
-static int programModeRegisters(mode_parameter_t *pModeParam, struct pll_value 
*pll)
+static int programModeRegisters(struct _mode_parameter_t *pModeParam, struct 
pll_value *pll)
 {
int ret = 0;
int cnt = 0;
@@ -198,7 +198,7 @@ static int programModeRegisters(mode_parameter_t 
*pModeParam, struct pll_value *
return ret;
 }
 
-int ddk750_setModeTiming(mode_parameter_t *parm, clock_type_t clock)
+int ddk750_setModeTiming(struct _mode_parameter_t *parm, clock_type_t clock)
 {
struct pll_value pll;
unsigned int uiActualPixelClk;
diff --git a/drivers/staging/sm750fb/ddk750_mode.h 
b/drivers/staging/sm750fb/ddk750_mode.h
index 6d204b8b4a01..5f8347d87800 100644
--- a/drivers/staging/sm750fb/ddk750_mode.h
+++ b/drivers/staging/sm750fb/ddk750_mode.h
@@ -3,37 +3,35 @@
 
 #include "ddk750_chip.h"
 
-typedef enum _spolarity_t {
+enum _spolarity_t {
POS = 0, /* positive */
NEG, /* negative */
-}
-spolarity_t;
+};
 
-typedef struct _mode_parameter_t {
+struct _mode_parameter_t {
/* Horizontal timing. */
unsigned long horizontal_total;
unsigned long horizontal_display_end;
unsigned long horizontal_sync_start;
unsigned long horizontal_sync_width;
-   spolarity_t horizontal_sync_polarity;
+   enum _spolarity_t horizontal_sync_polarity;
 
/* Vertical timing. */
unsigned long vertical_total;
unsigned long vertical_display_end;
unsigned long vertical_sync_start;
unsigned long vertical_sync_height;
-   spolarity_t vertical_sync_polarity;
+   enum _spolarity_t vertical_sync_polarity;
 
/* Refresh timing. */
unsigned long pixel_clock;
unsigned long horizontal_frequency;
-   unsigned long vertical_frequency;
+   enum _spolarity_t vertical_frequency;
 
/* Clock Phase. This clock phase only applies to Panel. */
-   spolarity_t clock_phase_polarity;
-}
-mode_parameter_t;
+   enum _spolarity_t clock_phase_polarity;
+};
 
-int ddk750_setModeTiming(mode_parameter_t *parm, clock_type_t clock);
+int ddk750_setModeTiming(struct _mode_parameter_t *parm, clock_type_t clock);
 
 #endif
diff --git a/drivers/staging/sm750fb/sm750_hw.c 
b/drivers/staging/sm750fb/sm750_hw.c
index fab3fc9c8330..4233e0bf12a7 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -252,7 +252,7 @@ int hw_sm750_crtc_setMode(struct lynxfb_crtc *crtc,
 {
int ret, fmt;
u32 reg;
-   mode_parameter_t modparm;
+   struct _mode_parameter_t modparm;
clock_type_t clock;
struct sm750_dev *sm750_dev;
struct lynxfb_par *par;
-- 
2.11.0

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


[PATCH v3 3/4] staging: sm750fb: Alignment should match open parenthesis

2017-03-13 Thread Arushi Singhal
Fix checkpatch issues: "CHECK: Alignment should match open parenthesis".

Signed-off-by: Arushi Singhal 
---
 drivers/staging/sm750fb/ddk750_mode.c | 79 +--
 1 file changed, 39 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c 
b/drivers/staging/sm750fb/ddk750_mode.c
index 45af806c8d55..eea5aef2956f 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -28,9 +28,9 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
poke32(CRT_AUTO_CENTERING_TL, 0);
 
poke32(CRT_AUTO_CENTERING_BR,
-   (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
-   CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
-   ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
+  (((y - 1) << CRT_AUTO_CENTERING_BR_BOTTOM_SHIFT) &
+   CRT_AUTO_CENTERING_BR_BOTTOM_MASK) |
+  ((x - 1) & CRT_AUTO_CENTERING_BR_RIGHT_MASK));
 
/*
 * Assume common fields in dispControl have been properly set before
@@ -72,8 +72,7 @@ static unsigned long 
displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 }
 
 /* only timing related registers will be  programed */
-static int programModeRegisters(mode_parameter_t *pModeParam,
-   struct pll_value *pll)
+static int programModeRegisters(mode_parameter_t *pModeParam, struct pll_value 
*pll)
 {
int ret = 0;
int cnt = 0;
@@ -83,32 +82,32 @@ static int programModeRegisters(mode_parameter_t 
*pModeParam,
/* programe secondary pixel clock */
poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
poke32(CRT_HORIZONTAL_TOTAL,
-   (((pModeParam->horizontal_total - 1) <<
-   CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
-   CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
-   ((pModeParam->horizontal_display_end - 1) &
-   CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
+  (((pModeParam->horizontal_total - 1) <<
+CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
+   CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
+  ((pModeParam->horizontal_display_end - 1) &
+   CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
 
poke32(CRT_HORIZONTAL_SYNC,
-   ((pModeParam->horizontal_sync_width <<
-   CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
-   CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
-   ((pModeParam->horizontal_sync_start - 1) &
-   CRT_HORIZONTAL_SYNC_START_MASK));
+  ((pModeParam->horizontal_sync_width <<
+CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
+   CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
+  ((pModeParam->horizontal_sync_start - 1) &
+   CRT_HORIZONTAL_SYNC_START_MASK));
 
poke32(CRT_VERTICAL_TOTAL,
-   (((pModeParam->vertical_total - 1) <<
-   CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
-   CRT_VERTICAL_TOTAL_TOTAL_MASK) |
-   ((pModeParam->vertical_display_end - 1) &
-   CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
+  (((pModeParam->vertical_total - 1) <<
+CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
+   CRT_VERTICAL_TOTAL_TOTAL_MASK) |
+  ((pModeParam->vertical_display_end - 1) &
+   CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
 
poke32(CRT_VERTICAL_SYNC,
-   ((pModeParam->vertical_sync_height <<
-   CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
-   CRT_VERTICAL_SYNC_HEIGHT_MASK) |
-   ((pModeParam->vertical_sync_start - 1) &
-   CRT_VERTICAL_SYNC_START_MASK));
+  ((pModeParam->vertical_sync_height <<
+CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
+   CRT_VERTICAL_SYNC_HEIGHT_MASK) |
+  ((pModeParam->vertical_sync_start - 1) &
+   CRT_VERTICAL_SYNC_START_MASK));
 
tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
if (pModeParam->vertical_sync_polarity)
@@ -140,25 +139,25 @@ static int programModeRegisters(mode_parameter_t 
*pModeParam,
poke32(PANEL_HORIZONTAL_TOTAL, reg);
 
poke32(PANEL_HORIZONTAL_SYNC,
-   ((pModeParam->horizontal_sync_width <<
-   PANEL_HORIZONTAL_SYNC_WIDTH_SHIFT) &
-   PANEL_HORIZONTAL_SYNC_WIDTH_MASK) |
-

[PATCH v3 1/4] staging: sm750fb: function prototype argument should have an identifier name

2017-03-13 Thread Arushi Singhal
function prototype arguments like 'struct vb_device_info *','unsigned
long' etc. should have an identifier name.

Signed-off-by: Arushi Singhal 
---
 changes in v3
 - add the identifier name of one more prototype arguments.

 drivers/staging/sm750fb/ddk750_display.h | 2 +-
 drivers/staging/sm750fb/ddk750_mode.h| 2 +-
 drivers/staging/sm750fb/ddk750_power.h   | 2 +-
 drivers/staging/sm750fb/sm750.h  | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_display.h 
b/drivers/staging/sm750fb/ddk750_display.h
index e2a3f84ca4c5..609bf742efff 100644
--- a/drivers/staging/sm750fb/ddk750_display.h
+++ b/drivers/staging/sm750fb/ddk750_display.h
@@ -102,6 +102,6 @@ typedef enum _disp_output_t {
 }
 disp_output_t;
 
-void ddk750_setLogicalDispOut(disp_output_t);
+void ddk750_setLogicalDispOut(disp_output_t output);
 
 #endif
diff --git a/drivers/staging/sm750fb/ddk750_mode.h 
b/drivers/staging/sm750fb/ddk750_mode.h
index 2183e664cf4b..6d204b8b4a01 100644
--- a/drivers/staging/sm750fb/ddk750_mode.h
+++ b/drivers/staging/sm750fb/ddk750_mode.h
@@ -34,6 +34,6 @@ typedef struct _mode_parameter_t {
 }
 mode_parameter_t;
 
-int ddk750_setModeTiming(mode_parameter_t *, clock_type_t);
+int ddk750_setModeTiming(mode_parameter_t *parm, clock_type_t clock);
 
 #endif
diff --git a/drivers/staging/sm750fb/ddk750_power.h 
b/drivers/staging/sm750fb/ddk750_power.h
index ec0b99d6a7ad..44c4fc587e96 100644
--- a/drivers/staging/sm750fb/ddk750_power.h
+++ b/drivers/staging/sm750fb/ddk750_power.h
@@ -14,7 +14,7 @@ DPMS_t;
   (peek32(MISC_CTRL) & ~MISC_CTRL_DAC_POWER_OFF) | (off)); \
 }
 
-void ddk750_set_dpms(DPMS_t);
+void ddk750_set_dpms(DPMS_t state);
 void sm750_set_power_mode(unsigned int powerMode);
 void sm750_set_current_gate(unsigned int gate);
 
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 306711ed55f9..5b186dafedec 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -184,8 +184,8 @@ static inline unsigned long ps_to_hz(unsigned int psvalue)
 }
 
 int hw_sm750_map(struct sm750_dev *sm750_dev, struct pci_dev *pdev);
-int hw_sm750_inithw(struct sm750_dev*, struct pci_dev *);
-void hw_sm750_initAccel(struct sm750_dev *);
+int hw_sm750_inithw(struct sm750_dev *sm750_dev, struct pci_dev *pdev);
+void hw_sm750_initAccel(struct sm750_dev *sm750_dev);
 int hw_sm750_deWait(void);
 int hw_sm750le_deWait(void);
 
-- 
2.11.0

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


[PATCH v3 2/4] staging: sm750fb: fixes add blank line after function/struct/union/enum declarations

2017-03-13 Thread Arushi Singhal
This patch fixes the warnings reported by checkpatch.pl
for please use a blank line after function/struct/union/enum
declarations.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/sm750fb/sm750_cursor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/sm750fb/sm750_cursor.c 
b/drivers/staging/sm750fb/sm750_cursor.c
index 612e9ab9d569..b64dc8a4a8fb 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -54,6 +54,7 @@ void sm750_hw_cursor_enable(struct lynx_cursor *cursor)
reg = (cursor->offset & HWC_ADDRESS_ADDRESS_MASK) | HWC_ADDRESS_ENABLE;
poke32(HWC_ADDRESS, reg);
 }
+
 void sm750_hw_cursor_disable(struct lynx_cursor *cursor)
 {
poke32(HWC_ADDRESS, 0);
@@ -65,6 +66,7 @@ void sm750_hw_cursor_setSize(struct lynx_cursor *cursor,
cursor->w = w;
cursor->h = h;
 }
+
 void sm750_hw_cursor_setPos(struct lynx_cursor *cursor,
int x, int y)
 {
@@ -74,6 +76,7 @@ void sm750_hw_cursor_setPos(struct lynx_cursor *cursor,
   (x & HWC_LOCATION_X_MASK);
poke32(HWC_LOCATION, reg);
 }
+
 void sm750_hw_cursor_setColor(struct lynx_cursor *cursor,
u32 fg, u32 bg)
 {
-- 
2.11.0

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


[PATCH v3 0/4] staging: sm750fb: fix multiple checkpatch warnings

2017-03-13 Thread Arushi Singhal
Fix multiple code styling warnings issued by checkpatch on files
in sm750fb driver 

Arushi Singhal (4):
  staging: sm750fb: function prototype argument should have an
identifier name
  staging: sm750fb: fixes add blank line after
function/struct/union/enum declarations
  staging: sm750fb: Alignment should match open parenthesis
  staging: sm750fb: Remove typedefs

 drivers/staging/sm750fb/ddk750_display.h |   2 +-
 drivers/staging/sm750fb/ddk750_mode.c|  83 
 drivers/staging/sm750fb/ddk750_mode.h|  20 +++---
 drivers/staging/sm750fb/ddk750_power.h   |   2 +-
 drivers/staging/sm750fb/sm750.h  |   4 +-
 drivers/staging/sm750fb/sm750_cursor.c   |   3 +
 drivers/staging/sm750fb/sm750_hw.c   |   2 +-
 7 files changed, 58 insertions(+), 58 deletions(-)

-- 
changes in v3
 - change the subject of the cover-patch.

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


[PATCH v2 2/2] staging: speakup: fix "Alignment match open parenthesis"

2017-03-13 Thread Arushi Singhal
Fix checkpatch issues: "CHECK: Alignment should match open parenthesis".

Signed-off-by: Arushi Singhal 
---
 drivers/staging/speakup/kobjects.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c 
b/drivers/staging/speakup/kobjects.c
index 8a586323b728..ca85476e3ff7 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -93,7 +93,7 @@ static void report_char_chartab_status(int reset, int 
received, int used,
} else if (received) {
len = snprintf(buf, sizeof(buf),
   " updated %d of %d %s\n",
-   used, received, object_type[do_characters]);
+  used, received, object_type[do_characters]);
if (rejected)
snprintf(buf + (len - 1), sizeof(buf) - (len - 1),
 " with %d reject%s\n",
-- 
2.11.0

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


[PATCH v2 1/2] staging: speakup: Added blank line after function/struct/union/enum declarations

2017-03-13 Thread Arushi Singhal
This patch fixes the warnings reported by checkpatch.pl
for please use a blank line after function/struct/union/enum
declarations.

Signed-off-by: Arushi Singhal 
---
 changes in v2
 - change the subject to make it more relevant.

 drivers/staging/speakup/main.c | 1 +
 drivers/staging/speakup/serialio.c | 1 +
 drivers/staging/speakup/speakup_dtlk.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 6786cfab7460..4b750ec8cd59 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -108,6 +108,7 @@ enum {
CT_Window,
CT_Max
 };
+
 #define read_all_mode CT_Max
 
 static struct tty_struct *tty;
diff --git a/drivers/staging/speakup/serialio.c 
b/drivers/staging/speakup/serialio.c
index aade52ee15a0..d51e382cb835 100644
--- a/drivers/staging/speakup/serialio.c
+++ b/drivers/staging/speakup/serialio.c
@@ -21,6 +21,7 @@ static void start_serial_interrupt(int irq);
 static const struct old_serial_port rs_table[] = {
SERIAL_PORT_DFNS
 };
+
 static const struct old_serial_port *serstate;
 static int timeouts;
 
diff --git a/drivers/staging/speakup/speakup_dtlk.c 
b/drivers/staging/speakup/speakup_dtlk.c
index eede2d82183a..c0b2208aa8a7 100644
--- a/drivers/staging/speakup/speakup_dtlk.c
+++ b/drivers/staging/speakup/speakup_dtlk.c
@@ -43,6 +43,7 @@ static int port_forced;
 static unsigned int synth_portlist[] = {
 0x25e, 0x29e, 0x2de, 0x31e, 0x35e, 0x39e, 0
 };
+
 static u_char synth_status;
 
 static struct var_t vars[] = {
-- 
2.11.0

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


[PATCH v2 0/2] staging: speakup: checkpatch cleanups

2017-03-13 Thread Arushi Singhal
Improve readability by fixing multiple checkpatch.pl
issues in speakup driver. 

Arushi Singhal (2):
  staging: speakup: Added blank line after function/struct/union/enum
declarations
  staging: speakup: fix "Alignment match open parenthesis"

 drivers/staging/speakup/kobjects.c | 2 +-
 drivers/staging/speakup/main.c | 1 +
 drivers/staging/speakup/serialio.c | 1 +
 drivers/staging/speakup/speakup_dtlk.c | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.11.0

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


[PATCH v3 2/2] staging: dvb-frontends: removed code in comments.

2017-03-13 Thread Arushi Singhal
Commenting out Code is a Bad Idea.
Comments are their to explain the code and how the code achieves its
goal and as codes in the comments  does not explain what the code is
doing so there is no use of commenting them.
So in this patch codes in the comments are removed.

Signed-off-by: Arushi Singhal 
---
 changes in v3
 - Improve the commit message

 drivers/media/dvb-frontends/drxk_hard.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/drxk_hard.c 
b/drivers/media/dvb-frontends/drxk_hard.c
index 7e1bbbaad625..2fe493768003 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -5283,7 +5283,6 @@ static int qam_set_symbolrate(struct drxk_state *state)
/* Select & calculate correct IQM rate */
adc_frequency = (state->m_sys_clock_freq * 1000) / 3;
ratesel = 0;
-   /* printk(KERN_DEBUG "drxk: SR %d\n", state->props.symbol_rate); */
if (state->props.symbol_rate <= 1188750)
ratesel = 3;
else if (state->props.symbol_rate <= 2377500)
-- 
2.11.0

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


[PATCH v3 1/2] staging: speakup: identation should use tabs

2017-03-13 Thread Arushi Singhal
Indentation should always use tabs and never spaces.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/speakup/speakup_dtlk.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/speakup/speakup_dtlk.h 
b/drivers/staging/speakup/speakup_dtlk.h
index b3b3cfc3db07..46d885fcfb20 100644
--- a/drivers/staging/speakup/speakup_dtlk.h
+++ b/drivers/staging/speakup/speakup_dtlk.h
@@ -24,11 +24,11 @@
 * usec later.
 */
 #define TTS_ALMOST_FULL0x08/* mask for AF bit: When set to 1,
-* indicates that less than 300 bytes
-* are available in the TTS input
-* buffer. AF is always 0 in the PCM,
-* TGN and CVSD modes.
-*/
+* indicates that less than 300 bytes
+* are available in the TTS input
+* buffer. AF is always 0 in the PCM,
+* TGN and CVSD modes.
+*/
 #define TTS_ALMOST_EMPTY 0x04  /* mask for AE bit: When set to 1,
 * indicates that less than 300 bytes
 * are remaining in DoubleTalk's input
-- 
2.11.0

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


[PATCH v3 0/2] staging: checkpatch cleanups

2017-03-13 Thread Arushi Singhal
Improve readability by fixing multiple checkpatch.pl
issues in drivers. 

Arushi Singhal (2):
  staging: speakup: identation should use tabs
  staging: dvb-frontends: removed code in comments.

 drivers/media/dvb-frontends/drxk_hard.c  |   1 -
 drivers/staging/speakup/speakup_dtlk.h   |  10 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

-- 
Changes since v3:
 -change the subject of cover letter.

2.11.0

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


[PATCH v2 2/2] staging: dvb-frontends: removed code in comments.

2017-03-13 Thread Arushi Singhal
Commenting out Code is a Bad Idea.
Comments are their to explain the code and how the code achieves its
goal and as codes in the comments  does not explain what the code is
doing so there is no use of commenting them.
So in this patch codes in the comments are removed.

Signed-off-by: Arushi Singhal 
---
 changes in v2
 - Improve the commit message

 drivers/media/dvb-frontends/drxk_hard.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/drxk_hard.c 
b/drivers/media/dvb-frontends/drxk_hard.c
index 7e1bbbaad625..2fe493768003 100644
--- a/drivers/media/dvb-frontends/drxk_hard.c
+++ b/drivers/media/dvb-frontends/drxk_hard.c
@@ -5283,7 +5283,6 @@ static int qam_set_symbolrate(struct drxk_state *state)
/* Select & calculate correct IQM rate */
adc_frequency = (state->m_sys_clock_freq * 1000) / 3;
ratesel = 0;
-   /* printk(KERN_DEBUG "drxk: SR %d\n", state->props.symbol_rate); */
if (state->props.symbol_rate <= 1188750)
ratesel = 3;
else if (state->props.symbol_rate <= 2377500)
-- 
2.11.0

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


[PATCH v2 1/2] staging: speakup: identation should use tabs

2017-03-13 Thread Arushi Singhal
Indentation should always use tabs and never spaces.

Signed-off-by: Arushi Singhal 
---
 drivers/staging/speakup/speakup_dtlk.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/speakup/speakup_dtlk.h 
b/drivers/staging/speakup/speakup_dtlk.h
index b3b3cfc3db07..46d885fcfb20 100644
--- a/drivers/staging/speakup/speakup_dtlk.h
+++ b/drivers/staging/speakup/speakup_dtlk.h
@@ -24,11 +24,11 @@
 * usec later.
 */
 #define TTS_ALMOST_FULL0x08/* mask for AF bit: When set to 1,
-* indicates that less than 300 bytes
-* are available in the TTS input
-* buffer. AF is always 0 in the PCM,
-* TGN and CVSD modes.
-*/
+* indicates that less than 300 bytes
+* are available in the TTS input
+* buffer. AF is always 0 in the PCM,
+* TGN and CVSD modes.
+*/
 #define TTS_ALMOST_EMPTY 0x04  /* mask for AE bit: When set to 1,
 * indicates that less than 300 bytes
 * are remaining in DoubleTalk's input
-- 
2.11.0

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


[PATCH v2 0/2] checkpatch cleanups

2017-03-13 Thread Arushi Singhal
Improve readability by fixing multiple checkpatch.pl
issues in speakup driver. 

Arushi Singhal (2):
  staging: speakup: identation should use tabs
  staging: dvb-frontends: removed code in comments.

 drivers/media/dvb-frontends/drxk_hard.c  |   1 -
 drivers/staging/speakup/speakup_dtlk.h   |  10 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.11.0

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


Re: [Outreachy kernel] [PATCH v2] staging: iio: ade7753: replace mlock with driver private lock

2017-03-13 Thread Alison Schofield
On Mon, Mar 13, 2017 at 10:01:07PM +0530, simran singhal wrote:
> The IIO subsystem is redefining iio_dev->mlock to be used by
> the IIO core only for protecting device operating mode changes.
> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
> 
> In this driver, mlock was being used to protect hardware state
> changes.  Replace it with a lock in the devices global data.
> 
> Fix some coding style issues related to white space also.
> 
> Signed-off-by: simran singhal 
> ---
> 
>  v2:
>-Removed new lock to reuse the existing lock
Simran,

The good news is that you have 2 patches that have similar
challenges!  I'll suggest picking one, drive it to completion,
then do the other.

This has the nested locking issue that Lars warned about.
Need to refactor to avoid.  Check back on his review comments.

I suggest dropping those whitespace changes - they appear
out of place in this patch since you are no longer actually
touching those lines of code.

alisons


> 
>  drivers/staging/iio/meter/ade7753.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7753.c 
> b/drivers/staging/iio/meter/ade7753.c
> index dfd8b71..d88eaa3 100644
> --- a/drivers/staging/iio/meter/ade7753.c
> +++ b/drivers/staging/iio/meter/ade7753.c
> @@ -83,10 +83,10 @@
>   * @buf_lock:   mutex to protect tx and rx
>   **/
>  struct ade7753_state {
> - struct spi_device   *us;
> - struct mutexbuf_lock;
> - u8  tx[ADE7753_MAX_TX] 
> cacheline_aligned;
> - u8  rx[ADE7753_MAX_RX];
> + struct spi_device   *us;
> + struct mutexbuf_lock;
> + u8  tx[ADE7753_MAX_TX] cacheline_aligned;
> + u8  rx[ADE7753_MAX_RX];
>  };
>  
>  static int ade7753_spi_write_reg_8(struct device *dev,
> @@ -484,7 +484,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
>   if (!val)
>   return -EINVAL;
>  
> - mutex_lock(_dev->mlock);
> + mutex_lock(>buf_lock);
>  
>   t = 27900 / val;
>   if (t > 0)
> @@ -505,7 +505,7 @@ static ssize_t ade7753_write_frequency(struct device *dev,
>   ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
>  
>  out:
> - mutex_unlock(_dev->mlock);
> + mutex_unlock(>buf_lock);
>  
>   return ret ? ret : len;
>  }
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170313163107.GA31496%40singhal-Inspiron-5558.
> For more options, visit https://groups.google.com/d/optout.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [Outreachy kernel] [PATCH v3] staging: adis16060_core: Use private driver lock instead of mlock

2017-03-13 Thread Alison Schofield
On Mon, Mar 13, 2017 at 11:41:30PM +0530, simran singhal wrote:
> The IIO subsystem is redefining iio_dev->mlock to be used by
> the IIO core only for protecting device operating mode changes.
> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
> 
> In this driver, mlock was being used to protect hardware state
> changes.  Replace it with a lock in the devices global data.
> 
> Signed-off-by: simran singhal 
> ---
>  
>  v3:
>-Removed new lock to reuse the existing lock

Hi Simran,

Check Lars review comments about refactoring into a
write_then_read function protected by same buf_lock.

As it is now, it'll deadlock on buf_lock. It locks in
_read_raw and then calls _spi_write where an attempt
is made to grab the same lock.

alisons


> 
>  drivers/staging/iio/gyro/adis16060_core.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/gyro/adis16060_core.c 
> b/drivers/staging/iio/gyro/adis16060_core.c
> index c9d46e7..602ec53 100644
> --- a/drivers/staging/iio/gyro/adis16060_core.c
> +++ b/drivers/staging/iio/gyro/adis16060_core.c
> @@ -83,11 +83,12 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>  {
>   u16 tval = 0;
>   int ret;
> + struct adis16060_state *st = iio_priv(indio_dev);
>  
>   switch (mask) {
>   case IIO_CHAN_INFO_RAW:
>   /* Take the iio_dev status lock */
> - mutex_lock(_dev->mlock);
> + mutex_lock(>buf_lock);
>   ret = adis16060_spi_write(indio_dev, chan->address);
>   if (ret < 0)
>   goto out_unlock;
> @@ -96,7 +97,7 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>   if (ret < 0)
>   goto out_unlock;
>  
> - mutex_unlock(_dev->mlock);
> + mutex_unlock(>buf_lock);
>   *val = tval;
>   return IIO_VAL_INT;
>   case IIO_CHAN_INFO_OFFSET:
> @@ -112,7 +113,7 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
>   return -EINVAL;
>  
>  out_unlock:
> - mutex_unlock(_dev->mlock);
> + mutex_unlock(>buf_lock);
>   return ret;
>  }
>  
> -- 
> 2.7.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170313181130.GA27111%40singhal-Inspiron-5558.
> For more options, visit https://groups.google.com/d/optout.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[RESEND PATCH v6 5/8] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2

2017-03-13 Thread Roy Pledge
From: Roy Pledge 

Add QBman APIs for frame queue and buffer pool operations.

Signed-off-by: Roy Pledge 
Signed-off-by: Haiying Wang 
Signed-off-by: Stuart Yoder 
---

Notes:
-v6
   -checkpatch: Add additional () in QBMAN_INDEX_FROM_DQRR
   -checkpatch: Replace check for NULL with ! operator
-v5
   -fixed typo that caused a compile error
-v4
   -adjust file location to be in drivers/staging
   -updated copyright
   -added definition for static dequeue token value
   -fixed bug in SDQCR #define
   -added missing #include guard in qbman-portal.h
   -added #define for QMAN_REV_MASK
   -whitespace, alignment cleanup
-v3
   -replace hardcoded dequeue token with a #define and check that
token when checking for a new result (bug fix suggested by
Ioana Radulescu)
-v2
   -fix bug in buffer release command, by setting bpid field
   -handle error (NULL) return value from qbman_swp_mc_complete()
   -error message cleanup
   -fix bug in sending management commands where the verb was
properly initialized

 drivers/staging/fsl-mc/bus/dpio/Makefile   |2 +-
 drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 1033 
 drivers/staging/fsl-mc/bus/dpio/qbman-portal.h |  469 +++
 3 files changed, 1503 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
 create mode 100644 drivers/staging/fsl-mc/bus/dpio/qbman-portal.h

diff --git a/drivers/staging/fsl-mc/bus/dpio/Makefile 
b/drivers/staging/fsl-mc/bus/dpio/Makefile
index 128befc..6588498 100644
--- a/drivers/staging/fsl-mc/bus/dpio/Makefile
+++ b/drivers/staging/fsl-mc/bus/dpio/Makefile
@@ -6,4 +6,4 @@ subdir-ccflags-y := -Werror
 
 obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o
 
-fsl-mc-dpio-objs := dpio.o
+fsl-mc-dpio-objs := dpio.o qbman-portal.o
diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c 
b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
new file mode 100644
index 000..c75f546
--- /dev/null
+++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
@@ -0,0 +1,1033 @@
+/*
+ * Copyright (C) 2014-2016 Freescale Semiconductor, Inc.
+ * Copyright 2016 NXP
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *   names of its contributors may be used to endorse or promote products
+ *   derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+#include 
+#include 
+#include "../../include/dpaa2-global.h"
+
+#include "qbman-portal.h"
+
+#define QMAN_REV_4000   0x0400
+#define QMAN_REV_4100   0x0401
+#define QMAN_REV_4101   0x04010001
+#define QMAN_REV_MASK   0x
+
+/* All QBMan command and result structures use this "valid bit" encoding */
+#define QB_VALID_BIT ((u32)0x80)
+
+/* QBMan portal management command codes */
+#define QBMAN_MC_ACQUIRE   0x30
+#define QBMAN_WQCHAN_CONFIGURE 0x46
+
+/* CINH register offsets */
+#define QBMAN_CINH_SWP_EQAR0x8c0
+#define QBMAN_CINH_SWP_DQPI0xa00
+#define QBMAN_CINH_SWP_DCAP0xac0
+#define QBMAN_CINH_SWP_SDQCR   0xb00
+#define QBMAN_CINH_SWP_RAR 0xcc0
+#define QBMAN_CINH_SWP_ISR 0xe00
+#define QBMAN_CINH_SWP_IER 0xe40
+#define QBMAN_CINH_SWP_ISDR0xe80
+#define 

[RESEND PATCH v6 6/8] bus: fsl-mc: dpio: add the DPAA2 DPIO service interface

2017-03-13 Thread Roy Pledge
From: Roy Pledge 

The DPIO service interface handles initialization of DPIO objects
and exports APIs to be used by other DPAA2 object drivers to perform
queuing and buffer management related operations.  The service allows
registration of callbacks when frames or notifications are received.

Signed-off-by: Roy Pledge 
Signed-off-by: Haiying Wang 
Signed-off-by: Stuart Yoder 
---

Notes:
-v6
   -checkpatch: Add comments for spinlock_t declarations
-v4
   -updated copyright
   -adjust file location to be in drivers/staging
   -updated copyright
   -added missing free on error path
   -fixed typo in comment
   -whitespace and alignment cleanup
-v3
   -zero memory allocated for a dpio store (bug fix suggested
by Ioana Radulescu)
-v2
   -use service_select_by_cpu() for re-arming DPIO interrupts
   -replace use of NR_CPUS with num_possible_cpus()

 drivers/staging/fsl-mc/bus/dpio/Makefile   |   2 +-
 drivers/staging/fsl-mc/bus/dpio/dpio-service.c | 618 +
 drivers/staging/fsl-mc/include/dpaa2-io.h  | 139 ++
 3 files changed, 758 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/fsl-mc/bus/dpio/dpio-service.c
 create mode 100644 drivers/staging/fsl-mc/include/dpaa2-io.h

diff --git a/drivers/staging/fsl-mc/bus/dpio/Makefile 
b/drivers/staging/fsl-mc/bus/dpio/Makefile
index 6588498..0778da7 100644
--- a/drivers/staging/fsl-mc/bus/dpio/Makefile
+++ b/drivers/staging/fsl-mc/bus/dpio/Makefile
@@ -6,4 +6,4 @@ subdir-ccflags-y := -Werror
 
 obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o
 
-fsl-mc-dpio-objs := dpio.o qbman-portal.o
+fsl-mc-dpio-objs := dpio.o qbman-portal.o dpio-service.o
diff --git a/drivers/staging/fsl-mc/bus/dpio/dpio-service.c 
b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
new file mode 100644
index 000..e5d6674
--- /dev/null
+++ b/drivers/staging/fsl-mc/bus/dpio/dpio-service.c
@@ -0,0 +1,618 @@
+/*
+ * Copyright 2014-2016 Freescale Semiconductor Inc.
+ * Copyright 2016 NXP
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *  notice, this list of conditions and the following disclaimer in the
+ *  documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *  names of its contributors may be used to endorse or promote products
+ *  derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include 
+#include "../../include/mc.h"
+#include "../../include/dpaa2-io.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "dpio.h"
+#include "qbman-portal.h"
+
+struct dpaa2_io {
+   atomic_t refs;
+   struct dpaa2_io_desc dpio_desc;
+   struct qbman_swp_desc swp_desc;
+   struct qbman_swp *swp;
+   struct list_head node;
+   /* protect against multiple management commands */
+   spinlock_t lock_mgmt_cmd;
+   /* protect notifications list */
+   spinlock_t lock_notifications;
+   struct list_head notifications;
+};
+
+struct dpaa2_io_store {
+   unsigned int max;
+   dma_addr_t paddr;
+   struct dpaa2_dq *vaddr;
+   void *alloced_addr;/* unaligned value from kmalloc() */
+   unsigned int idx;  /* position of the next-to-be-returned entry */
+   struct qbman_swp *swp; /* portal used to issue VDQCR */
+   struct device *dev;/* device used for DMA mapping */
+};
+
+/* keep a per cpu array of DPIOs for fast access */
+static 

  1   2   >