[PATCH] staging: dgnc: Let line have less 80 char

[Sean]

This patch fix a minor checkpath warming:

"WARNING: line over 80 characters"

Signed-off-by: Sean 
---
 drivers/staging/dgnc/dgnc_neo.c | 116 
 1 file changed, 82 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index ba57e95..37fb556 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -107,7 +107,7 @@ static inline void neo_set_cts_flow_control(struct 
channel_t *ch)
/* Turn off auto Xon flow control */
efr &= ~UART_17158_EFR_IXON;
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -143,7 +143,7 @@ static inline void neo_set_rts_flow_control(struct 
channel_t *ch)
ier &= ~UART_17158_IER_XOFF;
efr &= ~UART_17158_EFR_IXOFF;
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -181,7 +181,7 @@ static inline void neo_set_ixon_flow_control(struct 
channel_t *ch)
/* Turn on auto Xon flow control */
efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -219,7 +219,7 @@ static inline void neo_set_ixoff_flow_control(struct 
channel_t *ch)
ier |= UART_17158_IER_XOFF;
efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -260,7 +260,7 @@ static inline void neo_set_no_input_flow_control(struct 
channel_t *ch)
else
efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -298,7 +298,7 @@ static inline void neo_set_no_output_flow_control(struct 
channel_t *ch)
else
efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -403,7 +403,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
ch->ch_intr_rx++;
neo_copy_data_from_uart_to_queue(ch);
 
-   /* Call our tty layer to enforce queue flow control if 
needed. */
+   /* 
+* Call our tty layer to enforcer
+* queue flow control if needed.
+*/
spin_lock_irqsave(&ch->ch_lock, flags);
dgnc_check_queue_flow_control(ch);
spin_unlock_irqrestore(&ch->ch_lock, flags);
@@ -428,7 +431,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
 * one it was, so we can suspend or resume data flow.
 */
if (cause == UART_17158_XON_DETECT) {
-   /* Is output stopped right now, if so, resume 
it */
+   /*
+* Is output stopped right now,
+* if so, resume it
+*/
if (brd->channels[port]->ch_flags & CH_STOP) {
spin_lock_irqsave(&ch->ch_lock,
  flags);
@@ -449,8 +455,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
 
if (isr & UART_17158_IIR_HWFLOW_STATE_CHANGE) {
/*
-* If we get here, this means the hardware is doing 
auto flow control.
-* Check to see whether RTS/DTR or CTS/DSR caused this 
interrupt.
+* If we get here,
+ 

[PATCH] staging: dgnc: Let line have less 80 char

[Sean]

This patch fix a minor checkpath warming:

"WARNING: line over 80 characters"

Signed-off-by: Sean 
---
 drivers/staging/dgnc/dgnc_neo.c | 116 
 1 file changed, 82 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index ba57e95..37fb556 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -107,7 +107,7 @@ static inline void neo_set_cts_flow_control(struct 
channel_t *ch)
/* Turn off auto Xon flow control */
efr &= ~UART_17158_EFR_IXON;
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -143,7 +143,7 @@ static inline void neo_set_rts_flow_control(struct 
channel_t *ch)
ier &= ~UART_17158_IER_XOFF;
efr &= ~UART_17158_EFR_IXOFF;
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -181,7 +181,7 @@ static inline void neo_set_ixon_flow_control(struct 
channel_t *ch)
/* Turn on auto Xon flow control */
efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -219,7 +219,7 @@ static inline void neo_set_ixoff_flow_control(struct 
channel_t *ch)
ier |= UART_17158_IER_XOFF;
efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -260,7 +260,7 @@ static inline void neo_set_no_input_flow_control(struct 
channel_t *ch)
else
efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -298,7 +298,7 @@ static inline void neo_set_no_output_flow_control(struct 
channel_t *ch)
else
efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -403,7 +403,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
ch->ch_intr_rx++;
neo_copy_data_from_uart_to_queue(ch);
 
-   /* Call our tty layer to enforce queue flow control if 
needed. */
+   /* 
+* Call our tty layer to enforcer
+* queue flow control if needed.
+*/
spin_lock_irqsave(&ch->ch_lock, flags);
dgnc_check_queue_flow_control(ch);
spin_unlock_irqrestore(&ch->ch_lock, flags);
@@ -428,7 +431,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
 * one it was, so we can suspend or resume data flow.
 */
if (cause == UART_17158_XON_DETECT) {
-   /* Is output stopped right now, if so, resume 
it */
+   /*
+* Is output stopped right now,
+* if so, resume it
+*/
if (brd->channels[port]->ch_flags & CH_STOP) {
spin_lock_irqsave(&ch->ch_lock,
  flags);
@@ -449,8 +455,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
 
if (isr & UART_17158_IIR_HWFLOW_STATE_CHANGE) {
/*
-* If we get here, this means the hardware is doing 
auto flow control.
-* Check to see whether RTS/DTR or CTS/DSR caused this 
interrupt.
+* If we get here,
+ 

[PATCH] staging: dgnc: Let line have less 80 char

[Sean]

This patch fix a minor checkpath warming:

"WARNING: line over 80 characters"

Signed-off-by: Sean Wei 
---
 drivers/staging/dgnc/dgnc_neo.c | 116 
 1 file changed, 82 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index ba57e95..37fb556 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -107,7 +107,7 @@ static inline void neo_set_cts_flow_control(struct 
channel_t *ch)
/* Turn off auto Xon flow control */
efr &= ~UART_17158_EFR_IXON;
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -143,7 +143,7 @@ static inline void neo_set_rts_flow_control(struct 
channel_t *ch)
ier &= ~UART_17158_IER_XOFF;
efr &= ~UART_17158_EFR_IXOFF;
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -181,7 +181,7 @@ static inline void neo_set_ixon_flow_control(struct 
channel_t *ch)
/* Turn on auto Xon flow control */
efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -219,7 +219,7 @@ static inline void neo_set_ixoff_flow_control(struct 
channel_t *ch)
ier |= UART_17158_IER_XOFF;
efr |= (UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -260,7 +260,7 @@ static inline void neo_set_no_input_flow_control(struct 
channel_t *ch)
else
efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXOFF);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -298,7 +298,7 @@ static inline void neo_set_no_output_flow_control(struct 
channel_t *ch)
else
efr &= ~(UART_17158_EFR_ECB | UART_17158_EFR_IXON);
 
-   /* Why? Becuz Exar's spec says we have to zero it out before setting it 
*/
+   /* Becuz Exar's spec says we have to zero it out before setting it */
writeb(0, &ch->ch_neo_uart->efr);
 
/* Turn on UART enhanced bits */
@@ -403,7 +403,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
ch->ch_intr_rx++;
neo_copy_data_from_uart_to_queue(ch);
 
-   /* Call our tty layer to enforce queue flow control if 
needed. */
+   /* 
+* Call our tty layer to enforcer
+* queue flow control if needed.
+*/
spin_lock_irqsave(&ch->ch_lock, flags);
dgnc_check_queue_flow_control(ch);
spin_unlock_irqrestore(&ch->ch_lock, flags);
@@ -428,7 +431,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
 * one it was, so we can suspend or resume data flow.
 */
if (cause == UART_17158_XON_DETECT) {
-   /* Is output stopped right now, if so, resume 
it */
+   /*
+* Is output stopped right now,
+* if so, resume it
+*/
if (brd->channels[port]->ch_flags & CH_STOP) {
spin_lock_irqsave(&ch->ch_lock,
  flags);
@@ -449,8 +455,10 @@ static inline void neo_parse_isr(struct dgnc_board *brd, 
uint port)
 
if (isr & UART_17158_IIR_HWFLOW_STATE_CHANGE) {
/*
-* If we get here, this means the hardware is doing 
auto flow control.
-* Check to see whether RTS/DTR or CTS/DSR caused this 
interrupt.
+* If we get here,
+ 

Re: [PATCH 1/13] KVM: Add tlb_remote_flush_with_range callback in kvm_x86_ops

[Sean Christopherson]

On Mon, 2018-09-10 at 08:38 +, Tianyu Lan wrote:
> Add flush range call back in the kvm_x86_ops and platform can use it
> to register its associated function. The parameter "kvm_tlb_range"
> accepts a single range and flush list which contains a list of ranges.
> 
> Signed-off-by: Lan Tianyu 
> ---
>  arch/x86/include/asm/kvm_host.h | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e12916e7c2fb..dcdf8cc16388 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -402,6 +402,12 @@ struct kvm_mmu {
>   u64 pdptrs[4]; /* pae */
>  };
>  
> +struct kvm_tlb_range {
> + u64 start_gfn;
> + u64 end_gfn;

IMO this struct and all functions should pass around the number of pages
instead of end_gfn to avoid confusion as to whether end_gfn is inclusive
or exlusive.

> + struct list_head *flush_list;
> +};
> +
>  enum pmc_type {
>   KVM_PMC_GP = 0,
>   KVM_PMC_FIXED,
> @@ -991,6 +997,8 @@ struct kvm_x86_ops {
>  
>   void (*tlb_flush)(struct kvm_vcpu *vcpu, bool invalidate_gpa);
>   int  (*tlb_remote_flush)(struct kvm *kvm);
> + int  (*tlb_remote_flush_with_range)(struct kvm *kvm,
> + struct kvm_tlb_range *range);
>  
>   /*
>    * Flush any TLB entries associated with the given GVA.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

URGENT RESPONSE NEEDED

[Sean Kim.]

Hello my dear.

Did you receive my email message to you? Please, get back to me ASAP as the 
matter is becoming late.  Expecting your urgent response.

Sean.

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

[PATCH] remove unused variable driver_desc

[Sean Behan]

---
 drivers/staging/emxx_udc/emxx_udc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 3536c03ff523..741147a4f0fe 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -38,7 +38,6 @@ static struct gpio_desc *vbus_gpio;
 static int vbus_irq;
 
 static const char  driver_name[] = "emxx_udc";
-static const char  driver_desc[] = DRIVER_DESC;
 
 /*===*/
 /* Prototype */
-- 
2.29.2

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

[PATCH] staging: emxx_udc: remove unused variable driver_desc

[Sean Behan]

When building with W=1 (or however you found it), there is a warning
that this variable is unused.

It is not used so remove it to fix the warning.

Signed-off-by: Sean Behan 
---
 drivers/staging/emxx_udc/emxx_udc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 3536c03ff523..741147a4f0fe 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -38,7 +38,6 @@ static struct gpio_desc *vbus_gpio;
 static int vbus_irq;
 
 static const char  driver_name[] = "emxx_udc";
-static const char  driver_desc[] = DRIVER_DESC;
 
 /*===*/
 /* Prototype */
-- 
2.29.2

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

[PATCH] staging: emxx_udc: remove unused variable driver_desc

[Sean Behan]

When building with W=1, there is a warning that this variable is unused.

It is not used so remove it to fix the warning.

Thanks to nat...@kernel.org for helping me submit my first patch.

Signed-off-by: Sean Behan 
---
 drivers/staging/emxx_udc/emxx_udc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 3536c03ff523..741147a4f0fe 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -38,7 +38,6 @@ static struct gpio_desc *vbus_gpio;
 static int vbus_irq;
 
 static const char  driver_name[] = "emxx_udc";
-static const char  driver_desc[] = DRIVER_DESC;
 
 /*===*/
 /* Prototype */
-- 
2.29.2

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

[PATCH] staging: emxx_udc: remove unused variable driver_desc

[Sean Behan]

Signed-off-by: Sean Behan 
---
 drivers/staging/emxx_udc/emxx_udc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 3536c03ff523..741147a4f0fe 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -38,7 +38,6 @@ static struct gpio_desc *vbus_gpio;
 static int vbus_irq;
 
 static const char  driver_name[] = "emxx_udc";
-static const char  driver_desc[] = DRIVER_DESC;
 
 /*===*/
 /* Prototype */
-- 
2.29.2

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

Re: [PATCH v6 4/5] drm/bridge: anx7625: add HDCP support

[Sean Paul]

 FLASH_BUF_LEN, ident);
> +   if (ret < 0) {
> +   DRM_DEV_ERROR(dev, "read flash data fail!\n");
> +   return -EIO;
> +   }
> +
> +   if (ident[29] == 0xFF && ident[30] == 0xFF && ident[31] == 0xFF)
> +   return -EINVAL;
> +
> +   return 0;
> +}
> +
> +static int anx7625_hdcp_setting(struct anx7625_data *ctx)
> +{
> +   u8 bcap;
> +   int ret;
> +   struct device *dev = &ctx->client->dev;
> +
> +   ret = anx7625_hdcp_key_probe(ctx);
> +   if (ret) {
> +   DRM_DEV_DEBUG_DRIVER(dev, "disable HDCP by config\n");
> +   return anx7625_write_and(ctx, ctx->i2c.rx_p1_client,
> +0xee, 0x9f);
> +   }
> +
> +   anx7625_aux_dpcd_read(ctx, 0x06, 0x80, 0x28, 1, &bcap);
> +   if (!(bcap & 0x01)) {
> +   DRM_DEV_DEBUG_DRIVER(dev, "bcap(0x%x) not support HDCP 
> 1.4.\n",
> +bcap);
> +   return anx7625_write_and(ctx, ctx->i2c.rx_p1_client,
> +0xee, 0x9f);
> +   }
> +
> +   DRM_DEV_DEBUG_DRIVER(dev, "enable HDCP 1.4\n");
> +
> +   ret = anx7625_write_or(ctx, ctx->i2c.rx_p1_client, 0xee, 0x20);
> +
> +   /* Try auth flag */
> +   ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, 0xec, 0x10);
> +   /* Interrupt for DRM */
> +   ret |= anx7625_write_or(ctx, ctx->i2c.rx_p1_client, 0xff, 0x01);
> +   if (ret < 0)
> +   DRM_DEV_ERROR(dev, "fail to enable HDCP\n");
> +
> +   return ret;
> +}
> +
>  static void anx7625_dp_start(struct anx7625_data *ctx)
>  {
> int ret;
> @@ -643,6 +787,9 @@ static void anx7625_dp_start(struct anx7625_data *ctx)
> return;
> }
>
> +   /* HDCP config */
> +   anx7625_hdcp_setting(ctx);

You should really use the "Content Protection" property to
enable/disable HDCP instead of force-enabling it at all times.

Sean

> +
> if (ctx->pdata.is_dpi)
> ret = anx7625_dpi_config(ctx);
> else
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h 
> b/drivers/gpu/drm/bridge/analogix/anx7625.h
> index beee95da2155..c6f93e4df0ed 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.h
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
> @@ -154,9 +154,45 @@
>
>  #define  I2C_ADDR_7E_FLASH_CONTROLLER  0x7E
>
> +#define FLASH_SRAM_SEL  0x00
> +#define SRAM_ADDR_HIGH  0x01
> +#define SRAM_ADDR_LOW   0x02
> +#define SRAM_LEN_HIGH   0x03
> +#define SRAM_LEN_LOW0x04
>  #define FLASH_LOAD_STA  0x05
>  #define FLASH_LOAD_STA_CHK BIT(7)
>
> +#define R_RAM_CTRL  0x05
> +/* bit positions */
> +#define FLASH_DONE  BIT(7)
> +#define BOOT_LOAD_DONE  BIT(6)
> +#define CRC_OK  BIT(5)
> +#define LOAD_DONE   BIT(4)
> +#define O_RW_DONE   BIT(3)
> +#define FUSE_BUSY   BIT(2)
> +#define DECRYPT_EN  BIT(1)
> +#define LOAD_START  BIT(0)
> +
> +#define FLASH_ADDR_HIGH 0x0F
> +#define FLASH_ADDR_LOW  0x10
> +#define FLASH_LEN_HIGH  0x31
> +#define FLASH_LEN_LOW   0x32
> +
> +#define R_FLASH_RW_CTRL 0x33
> +/* bit positions */
> +#define READ_DELAY_SELECT   BIT(7)
> +#define GENERAL_INSTRUCTION_EN  BIT(6)
> +#define FLASH_ERASE_EN  BIT(5)
> +#define RDID_READ_ENBIT(4)
> +#define REMS_READ_ENBIT(3)
> +#define WRITE_STATUS_EN BIT(2)
> +#define FLASH_READ  BIT(1)
> +#define FLASH_WRITE BIT(0)
> +
> +#define FLASH_BUF_BASE_ADDR 0x60
> +#define FLASH_BUF_LEN   0x20
> +#define FLASH_KEY_OFFSET0x8000
> +
>  #define  XTAL_FRQ_SEL0x3F
>  /* bit field positions */
>  #define  XTAL_FRQ_SEL_POS5
> --
> 2.25.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 4/5] drm/bridge: anx7625: add HDCP support

[Sean Paul]

On Mon, Mar 29, 2021 at 6:27 AM Xin Ji  wrote:
>
> On Thu, Mar 25, 2021 at 02:19:23PM -0400, Sean Paul wrote:
> > On Fri, Mar 19, 2021 at 2:35 AM Xin Ji  wrote:
> > >
> > > Add HDCP feature, enable HDCP function through chip internal key
> > > and downstream's capability.
> > >
> > > Signed-off-by: Xin Ji 
> > > ---

/snip

> > >  static void anx7625_dp_start(struct anx7625_data *ctx)
> > >  {
> > > int ret;
> > > @@ -643,6 +787,9 @@ static void anx7625_dp_start(struct anx7625_data *ctx)
> > > return;
> > > }
> > >
> > > +   /* HDCP config */
> > > +   anx7625_hdcp_setting(ctx);
> >
> > You should really use the "Content Protection" property to
> > enable/disable HDCP instead of force-enabling it at all times.
> >
> > Sean
> Hi Sean, it's hard to implement "Content Protection" property, we have
> implemented HDCP in firmware, it is not compatible with it. We don't
> have interface to get Downstream Cert.
> Thanks,
> Xin

Hi Xin,
I'm sorry, I don't understand what you mean when you say you don't
have an interface to get Downstream Cert.

The Content Protection property is just a means through which
userspace can turn on and turn off HDCP when it needs. As far as I can
tell, your patch turns on HDCP when the display is enabled and leaves
it on until it is disabled. This is undesirable since it forces HDCP
on the user.

Is it impossible to enable/disable HDCP outside of display
enable/disable on your hardware?

Thanks,

Sean

> >
> > > +
> > > if (ctx->pdata.is_dpi)
> > > ret = anx7625_dpi_config(ctx);
> > > else

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

Re: [PATCH] dma: mediatek: addres minor style issues flagged by clang-format.

[Sean Wang]

On Sun, Mar 17, 2019 at 1:45 AM Armando Miraglia  wrote:
>
> Running clang-format on mtk-hsdma.c I noticed that few lines would not
> need to be wrapped, since they fit 80 columns. This change changes such
> lines to better fit the style-guide.
>
> Signed-off-by: Armando Miraglia 
> ---
>  drivers/staging/mt7621-dma/mtk-hsdma.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c 
> b/drivers/staging/mt7621-dma/mtk-hsdma.c
> index 97571f1d697b..74c374f7bef4 100644
> --- a/drivers/staging/mt7621-dma/mtk-hsdma.c
> +++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
> @@ -264,8 +264,7 @@ static void mtk_hsdma_reset(struct mtk_hsdam_engine 
> *hsdma,
> /* init desc value */
> for (i = 0; i < HSDMA_DESCS_NUM; i++) {
> chan->tx_ring[i].addr0 = 0;
> -   chan->tx_ring[i].flags = HSDMA_DESC_LS0 |
> -   HSDMA_DESC_DONE;
> +   chan->tx_ring[i].flags = HSDMA_DESC_LS0 | HSDMA_DESC_DONE;
> }
> for (i = 0; i < HSDMA_DESCS_NUM; i++) {
> chan->rx_ring[i].addr0 = 0;
> @@ -435,8 +434,7 @@ static irqreturn_t mtk_hsdma_irq(int irq, void *devid)
> if (likely(status & HSDMA_INT_RX_Q0))
> tasklet_schedule(&hsdma->task);
> else
> -   dev_dbg(hsdma->ddev.dev, "unhandle irq status %08x\n",
> -   status);
> +   dev_dbg(hsdma->ddev.dev, "unhandle irq status %08x\n", 
> status);
> /* clean intr bits */
> mtk_hsdma_write(hsdma, HSDMA_REG_INT_STATUS, status);
>

Acked-by: Sean Wang 

> --
> 2.21.0.360.g471c308f928-goog
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 03/10] KVM: Prepare kvm_is_reserved_pfn() for PG_reserved changes

[Sean Christopherson]

On Tue, Nov 05, 2019 at 11:02:46AM +0100, David Hildenbrand wrote:
> On 05.11.19 10:49, David Hildenbrand wrote:
> >On 05.11.19 10:17, David Hildenbrand wrote:
> >>On 05.11.19 05:38, Dan Williams wrote:
> >>>On Thu, Oct 24, 2019 at 5:11 AM David Hildenbrand  wrote:
> 
> Right now, ZONE_DEVICE memory is always set PG_reserved. We want to
> change that.
> 
> KVM has this weird use case that you can map anything from /dev/mem
> into the guest. pfn_valid() is not a reliable check whether the memmap
> was initialized and can be touched. pfn_to_online_page() makes sure
> that we have an initialized memmap (and don't have ZONE_DEVICE memory).
> 
> Rewrite kvm_is_reserved_pfn() to make sure the function produces the
> same result once we stop setting ZONE_DEVICE pages PG_reserved.
> 
> Cc: Paolo Bonzini 
> Cc: "Radim Krčmář" 
> Cc: Michal Hocko 
> Cc: Dan Williams 
> Cc: KarimAllah Ahmed 
> Signed-off-by: David Hildenbrand 
> ---
> virt/kvm/kvm_main.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e9eb666eb6e8..9d18cc67d124 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -151,9 +151,15 @@ __weak int 
> kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,
> 
> bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
> {
> -   if (pfn_valid(pfn))
> -   return PageReserved(pfn_to_page(pfn));
> +   struct page *page = pfn_to_online_page(pfn);
> 
> +   /*
> +* We treat any pages that are not online (not managed by the 
> buddy)
> +* as reserved - this includes ZONE_DEVICE pages and pages without
> +* a memmap (e.g., mapped via /dev/mem).
> +*/
> +   if (page)
> +   return PageReserved(page);
>    return true;
> }
> >>>
> >>>So after this all the pfn_valid() usage in kvm_main.c is replaced with
> >>>pfn_to_online_page()? Looks correct to me.
> >>>
> >>>However, I'm worried that kvm is taking reference on ZONE_DEVICE pages
> >>>through some other path resulting in this:
> >>>
> >>>   
> >>> https://lore.kernel.org/linux-nvdimm/20190919154708.ga24...@angband.pl/
> >>>
> >>>I'll see if this patch set modulates or maintains that failure mode.
> >>>
> >>
> >>I'd assume that the behavior is unchanged. Ithink we get a reference to
> >>these ZONE_DEVICE pages via __get_user_pages_fast() and friends in
> >>hva_to_pfn_fast() and friends in virt/kvm/kvm_main.c
> >>
> >
> >I think I know what's going wrong:
> >
> >Pages that are pinned via gfn_to_pfn() and friends take a references,
> >however are often released via
> >kvm_release_pfn_clean()/kvm_release_pfn_dirty()/kvm_release_page_clean()...
> >
> >
> >E.g., in arch/x86/kvm/x86.c:reexecute_instruction()
> >
> >...
> >pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa));
> >...
> >kvm_release_pfn_clean(pfn);
> >
> >
> >
> >void kvm_release_pfn_clean(kvm_pfn_t pfn)
> >{
> > if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn))
> > put_page(pfn_to_page(pfn));
> >}
> >
> >This function makes perfect sense as the counterpart for kvm_get_pfn():
> >
> >void kvm_get_pfn(kvm_pfn_t pfn)
> >{
> > if (!kvm_is_reserved_pfn(pfn))
> > get_page(pfn_to_page(pfn));
> >}
> >
> >
> >As all ZONE_DEVICE pages are currently reserved, pages pinned via
> >gfn_to_pfn() and friends will often not see a put_page() AFAIKS.

Assuming gup() takes a reference for ZONE_DEVICE pages, yes, this is a
KVM bug.

> >Now, my patch does not change that, the result of
> >kvm_is_reserved_pfn(pfn) will be unchanged. A proper fix for that would
> >probably be
> >
> >a) To drop the reference to ZONE_DEVICE pages in gfn_to_pfn() and
> >friends, after you successfully pinned the pages. (not sure if that's
> >the right thing to do but you're the expert)
> >
> >b) To not use kvm_release_pfn_clean() and friends on pages that were
> >definitely pinned.

This is already KVM's intent, i.e. the purpose of the PageReserved() check
is simply to avoid putting a non-existent reference.  The problem is that
KVM assumes pages with PG_reserved set are never pinned, which AFAICT was
true when the code was first added.

> (talking to myself, sorry)
> 
> Thinking again, dropping this patch from this series could effectively also
> fix that issue. E.g., kvm_release_pfn_clean() and friends would always do a
> put_page() if "pfn_valid() and !PageReserved()", so after patch 9 also on
> ZONDE_DEVICE pages.

Yeah, this appears to be the correct fix.

> But it would have side effects that might not be desired. E.g.,:
> 
> 1. kvm_pfn_to_page() would also return ZONE_DEVICE pages (might even be the
> right thing to do).

This should be ok, at least on x86.  There are only three users of
kvm_pfn_to_page().  Two of those are on allocations that are contro

Re: [PATCH v1 03/10] KVM: Prepare kvm_is_reserved_pfn() for PG_reserved changes

[Sean Christopherson]

On Tue, Nov 05, 2019 at 09:30:53PM +0100, David Hildenbrand wrote:
> >>>I think I know what's going wrong:
> >>>
> >>>Pages that are pinned via gfn_to_pfn() and friends take a references,
> >>>however are often released via
> >>>kvm_release_pfn_clean()/kvm_release_pfn_dirty()/kvm_release_page_clean()...
> >>>
> >>>
> >>>E.g., in arch/x86/kvm/x86.c:reexecute_instruction()
> >>>
> >>>...
> >>>pfn = gfn_to_pfn(vcpu->kvm, gpa_to_gfn(gpa));
> >>>...
> >>>kvm_release_pfn_clean(pfn);
> >>>
> >>>
> >>>
> >>>void kvm_release_pfn_clean(kvm_pfn_t pfn)
> >>>{
> >>>   if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn))
> >>>   put_page(pfn_to_page(pfn));
> >>>}
> >>>
> >>>This function makes perfect sense as the counterpart for kvm_get_pfn():
> >>>
> >>>void kvm_get_pfn(kvm_pfn_t pfn)
> >>>{
> >>>   if (!kvm_is_reserved_pfn(pfn))
> >>>   get_page(pfn_to_page(pfn));
> >>>}
> >>>
> >>>
> >>>As all ZONE_DEVICE pages are currently reserved, pages pinned via
> >>>gfn_to_pfn() and friends will often not see a put_page() AFAIKS.
> >
> >Assuming gup() takes a reference for ZONE_DEVICE pages, yes, this is a
> >KVM bug.
> 
> Yes, it does take a reference AFAIKs. E.g.,
> 
> mm/gup.c:gup_pte_range():
> ...
>   if (pte_devmap(pte)) {
>   if (unlikely(flags & FOLL_LONGTERM))
>   goto pte_unmap;
> 
>   pgmap = get_dev_pagemap(pte_pfn(pte), pgmap);
>   if (unlikely(!pgmap)) {
>   undo_dev_pagemap(nr, nr_start, pages);
>   goto pte_unmap;
>   }
>   } else if (pte_special(pte))
>   goto pte_unmap;
> 
>   VM_BUG_ON(!pfn_valid(pte_pfn(pte)));
>   page = pte_page(pte);
> 
>   head = try_get_compound_head(page, 1);
> 
> try_get_compound_head() will increment the reference count.

Doh, I looked right at that code and somehow didn't connect the dots.
Thanks!

> >>>Now, my patch does not change that, the result of
> >>>kvm_is_reserved_pfn(pfn) will be unchanged. A proper fix for that would
> >>>probably be
> >>>
> >>>a) To drop the reference to ZONE_DEVICE pages in gfn_to_pfn() and
> >>>friends, after you successfully pinned the pages. (not sure if that's
> >>>the right thing to do but you're the expert)
> >>>
> >>>b) To not use kvm_release_pfn_clean() and friends on pages that were
> >>>definitely pinned.
> >
> >This is already KVM's intent, i.e. the purpose of the PageReserved() check
> >is simply to avoid putting a non-existent reference.  The problem is that
> >KVM assumes pages with PG_reserved set are never pinned, which AFAICT was
> >true when the code was first added.
> >
> >>(talking to myself, sorry)
> >>
> >>Thinking again, dropping this patch from this series could effectively also
> >>fix that issue. E.g., kvm_release_pfn_clean() and friends would always do a
> >>put_page() if "pfn_valid() and !PageReserved()", so after patch 9 also on
> >>ZONDE_DEVICE pages.
> >
> >Yeah, this appears to be the correct fix.
> >
> >>But it would have side effects that might not be desired. E.g.,:
> >>
> >>1. kvm_pfn_to_page() would also return ZONE_DEVICE pages (might even be the
> >>right thing to do).
> >
> >This should be ok, at least on x86.  There are only three users of
> >kvm_pfn_to_page().  Two of those are on allocations that are controlled by
> >KVM and are guaranteed to be vanilla MAP_ANONYMOUS.  The third is on guest
> >memory when running a nested guest, and in that case supporting ZONE_DEVICE
> >memory is desirable, i.e. KVM should play nice with a guest that is backed
> >by ZONE_DEVICE memory.
> >
> >>2. kvm_set_pfn_dirty() would also set ZONE_DEVICE pages dirty (might be
> >>okay)
> >
> >This is ok from a KVM perspective.
> 
> What about
> 
> void kvm_get_pfn(kvm_pfn_t pfn)
> {
>   if (!kvm_is_reserved_pfn(pfn))
>   get_page(pfn_to_page(pfn));
> }
> 
> Is a pure get_page() sufficient in case of ZONE_DEVICE?
> (asking because of the live references obtained via
> get_dev_pagemap(pte_pfn(pte), pgmap) in mm/gup.c:gup_pte_range() somewhat
> confuse me :) )

This ties into my concern with thp_adjust().  On x86, kvm_get_pfn() is
only used in two flows, to manually get a ref for VM_IO/VM_PFNMAP pages
and to switch the ref when mapping a non-hugetlbfs compound page, i.e. a
THP.

I assume VM_IO and PFNMAP can't apply to ZONE_DEVICE pages.

In the thp_adjust() case, when a THP is encountered and the original PFN
is for a non-PG_head page, KVM transfers the reference to the associated
PG_head page[*] and maps the associated 2mb chunk/page.  This is where KVM
uses kvm_get_pfn() and could run afoul of the get_dev_pagemap() refcounts.


[*] Technically I don't think it's guaranteed to be a PG_head, e.g. if the
THP is a 1gb page, as KVM currently only maps THP as 2mb pages.  But
the idea is the same, transfer the refcount the PFN that's actually
going into 

Re: [PATCH v1 03/10] KVM: Prepare kvm_is_reserved_pfn() for PG_reserved changes

[Sean Christopherson]

On Tue, Nov 05, 2019 at 03:02:40PM -0800, Dan Williams wrote:
> On Tue, Nov 5, 2019 at 12:31 PM David Hildenbrand  wrote:
> > > The scarier code (for me) is transparent_hugepage_adjust() and
> > > kvm_mmu_zap_collapsible_spte(), as I don't at all understand the
> > > interaction between THP and _PAGE_DEVMAP.
> >
> > The x86 KVM MMU code is one of the ugliest code I know (sorry, but it
> > had to be said :/ ). Luckily, this should be independent of the
> > PG_reserved thingy AFAIKs.
> 
> Both transparent_hugepage_adjust() and kvm_mmu_zap_collapsible_spte()
> are honoring kvm_is_reserved_pfn(), so again I'm missing where the
> page count gets mismanaged and leads to the reported hang.

When mapping pages into the guest, KVM gets the page via gup(), which
increments the page count for ZONE_DEVICE pages.  But KVM puts the page
using kvm_release_pfn_clean(), which skips put_page() if PageReserved()
and so never puts its reference to ZONE_DEVICE pages.

My transparent_hugepage_adjust() and kvm_mmu_zap_collapsible_spte()
comments were for a post-patch/series scenario wheren PageReserved() is
no longer true for ZONE_DEVICE pages.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 03/10] KVM: Prepare kvm_is_reserved_pfn() for PG_reserved changes

[Sean Christopherson]

On Tue, Nov 05, 2019 at 03:30:00PM -0800, Dan Williams wrote:
> On Tue, Nov 5, 2019 at 3:13 PM Sean Christopherson
>  wrote:
> >
> > On Tue, Nov 05, 2019 at 03:02:40PM -0800, Dan Williams wrote:
> > > On Tue, Nov 5, 2019 at 12:31 PM David Hildenbrand  
> > > wrote:
> > > > > The scarier code (for me) is transparent_hugepage_adjust() and
> > > > > kvm_mmu_zap_collapsible_spte(), as I don't at all understand the
> > > > > interaction between THP and _PAGE_DEVMAP.
> > > >
> > > > The x86 KVM MMU code is one of the ugliest code I know (sorry, but it
> > > > had to be said :/ ). Luckily, this should be independent of the
> > > > PG_reserved thingy AFAIKs.
> > >
> > > Both transparent_hugepage_adjust() and kvm_mmu_zap_collapsible_spte()
> > > are honoring kvm_is_reserved_pfn(), so again I'm missing where the
> > > page count gets mismanaged and leads to the reported hang.
> >
> > When mapping pages into the guest, KVM gets the page via gup(), which
> > increments the page count for ZONE_DEVICE pages.  But KVM puts the page
> > using kvm_release_pfn_clean(), which skips put_page() if PageReserved()
> > and so never puts its reference to ZONE_DEVICE pages.
> 
> Oh, yeah, that's busted.
> 
> > My transparent_hugepage_adjust() and kvm_mmu_zap_collapsible_spte()
> > comments were for a post-patch/series scenario wheren PageReserved() is
> > no longer true for ZONE_DEVICE pages.
> 
> Ah, ok, for that David is preserving kvm_is_reserved_pfn() returning
> true for ZONE_DEVICE because pfn_to_online_page() will fail for
> ZONE_DEVICE.

But David's proposed fix for the above refcount bug is to omit the patch
so that KVM no longer treats ZONE_DEVICE pages as reserved.  That seems
like the right thing to do, including for thp_adjust(), e.g. it would
naturally let KVM use 2mb pages for the guest when a ZONE_DEVICE page is
mapped with a huge page (2mb or above) in the host.  The only hiccup is
figuring out how to correctly transfer the reference.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 03/10] KVM: Prepare kvm_is_reserved_pfn() for PG_reserved changes

[Sean Christopherson]

On Tue, Nov 05, 2019 at 03:43:29PM -0800, Dan Williams wrote:
> On Tue, Nov 5, 2019 at 3:30 PM Dan Williams  wrote:
> >
> > On Tue, Nov 5, 2019 at 3:13 PM Sean Christopherson
> >  wrote:
> > >
> > > On Tue, Nov 05, 2019 at 03:02:40PM -0800, Dan Williams wrote:
> > > > On Tue, Nov 5, 2019 at 12:31 PM David Hildenbrand  
> > > > wrote:
> > > > > > The scarier code (for me) is transparent_hugepage_adjust() and
> > > > > > kvm_mmu_zap_collapsible_spte(), as I don't at all understand the
> > > > > > interaction between THP and _PAGE_DEVMAP.
> > > > >
> > > > > The x86 KVM MMU code is one of the ugliest code I know (sorry, but it
> > > > > had to be said :/ ). Luckily, this should be independent of the
> > > > > PG_reserved thingy AFAIKs.
> > > >
> > > > Both transparent_hugepage_adjust() and kvm_mmu_zap_collapsible_spte()
> > > > are honoring kvm_is_reserved_pfn(), so again I'm missing where the
> > > > page count gets mismanaged and leads to the reported hang.
> > >
> > > When mapping pages into the guest, KVM gets the page via gup(), which
> > > increments the page count for ZONE_DEVICE pages.  But KVM puts the page
> > > using kvm_release_pfn_clean(), which skips put_page() if PageReserved()
> > > and so never puts its reference to ZONE_DEVICE pages.
> >
> > Oh, yeah, that's busted.
> 
> Ugh, it's extra busted because every other gup user in the kernel
> tracks the pages resulting from gup and puts them (put_page()) when
> they are done. KVM wants to forget about whether it did a gup to get
> the page and optionally trigger put_page() based purely on the pfn.
> Outside of VFIO device assignment that needs pages pinned for DMA, why
> does KVM itself need to pin pages? If pages are pinned over a return
> to userspace that needs to be a FOLL_LONGTERM gup.

Short answer, KVM pins the page to ensure correctness with respect to the
primary MMU invalidating the associated host virtual address, e.g. when
the page is being migrated or unmapped from host userspace.

The main use of gup() is to handle guest page faults and map pages into
the guest, i.e. into KVM's secondary MMU.  KVM uses gup() to both get the
PFN and to temporarily pin the page.  The pin is held just long enough to
guaranteed that any invalidation via the mmu_notifier will be stalled
until after KVM finishes installing the page into the secondary MMU, i.e.
the pin is short-term and not held across a return to userspace or entry
into the guest.  When a subsequent mmu_notifier invalidation occurs, KVM
pulls the PFN from the secondary MMU and uses that to update accessed
and dirty bits in the host.

There are a few other KVM flows that eventually call into gup(), but those
are "traditional" short-term pins and use put_page() directly.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 03/10] KVM: Prepare kvm_is_reserved_pfn() for PG_reserved changes

[Sean Christopherson]

On Wed, Nov 06, 2019 at 07:56:34AM +0100, David Hildenbrand wrote:
> On 06.11.19 01:08, Dan Williams wrote:
> >On Tue, Nov 5, 2019 at 4:03 PM Sean Christopherson
> >>But David's proposed fix for the above refcount bug is to omit the patch
> >>so that KVM no longer treats ZONE_DEVICE pages as reserved.  That seems
> >>like the right thing to do, including for thp_adjust(), e.g. it would
> >>naturally let KVM use 2mb pages for the guest when a ZONE_DEVICE page is
> >>mapped with a huge page (2mb or above) in the host.  The only hiccup is
> >>figuring out how to correctly transfer the reference.
> >
> >That might not be the only hiccup. There's currently no such thing as
> >huge pages for ZONE_DEVICE, there are huge *mappings* (pmd and pud),
> >but the result of pfn_to_page() on such a mapping does not yield a
> >huge 'struct page'. It seems there are other paths in KVM that assume
> >that more typical page machinery is active like SetPageDirty() based
> >on kvm_is_reserved_pfn(). While I told David that I did not want to
> >see more usage of is_zone_device_page(), this patch below (untested)
> >seems a cleaner path with less surprises:
> >
> >diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> >index 4df0aa6b8e5c..fbea17c1810c 100644
> >--- a/virt/kvm/kvm_main.c
> >+++ b/virt/kvm/kvm_main.c
> >@@ -1831,7 +1831,8 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);
> >
> >  void kvm_release_pfn_clean(kvm_pfn_t pfn)
> >  {
> >-   if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn))
> >+   if ((!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn)) ||

The is_error_noslot_pfn() check shouldn't be overriden by zone_device.

> >+   (pfn_valid(pfn) && is_zone_device_page(pfn_to_page(pfn

But rather than special case kvm_release_pfn_clean(), I'd rather KVM
explicitly handle ZONE_DEVICE pages, there are other flows where KVM
really should be aware of ZONE_DEVICE pages, e.g. for sanity checks and
whatnot.  There are surprisingly few callers of kvm_is_reserved_pfn(), so
it's actually not too big of a change. 

> > put_page(pfn_to_page(pfn));
> >  }
> >  EXPORT_SYMBOL_GPL(kvm_release_pfn_clean);
> 
> I had the same thought, but I do wonder about the kvm_get_pfn() users,
> e.g.,:
> 
> hva_to_pfn_remapped():
>   r = follow_pfn(vma, addr, &pfn);
>   ...
>   kvm_get_pfn(pfn);
>   ...
> 
> We would not take a reference for ZONE_DEVICE, but later drop one reference
> via kvm_release_pfn_clean(). IOW, kvm_get_pfn() gets *really* dangerous to
> use. I can't tell if this can happen right now.
> 
> We do have 3 users of kvm_get_pfn() that we have to audit before this
> change. Also, we should add a comment to kvm_get_pfn() that it should never
> be used with possible ZONE_DEVICE pages.
> 
> Also, we should add a comment to kvm_release_pfn_clean(), describing why we
> treat ZONE_DEVICE in a special way here.
> 
> 
> We can then progress like this
> 
> 1. Get this fix upstream, it's somewhat unrelated to this series.
> 2. This patch here remains as is in this series (+/- documentation update)
> 3. Long term, rework KVM to not have to not treat ZONE_DEVICE like reserved
> pages. E.g., get rid of kvm_get_pfn(). Then, this special zone check can go.

Dropping kvm_get_pfn() is less than ideal, and at this point unnecessary.
I'm 99% sure the existing call sites for kvm_get_pfn() can never be
reached with ZONE_DEVICE pages.  I think we can do:

  1. Get a fix upstream to have KVM stop treating ZONE_DEVICE pages as
 reserved PFNs, i.e. exempt them in kvm_is_reserved_pfn() and change
 the callers of kvm_is_reserved_pfn() to handle ZONE_DEVICE pages.
  2. Drop this patch from the series, and instead remove the special
 treatment of ZONE_DEVICE pages from kvm_is_reserved_pfn().

Give me a few minutes to prep a patch.

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

[PATCH] Staging: slicoss: Fix long line issues in slicoss.c

[Sean Cleator]

 A patch to the slicoss.c file to fix some of the long line issues found by the
 checkpath.pl tool 
 Signed-off-by: Sean Cleator 

---
 drivers/staging/slicoss/slicoss.c | 39 ++-
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c 
b/drivers/staging/slicoss/slicoss.c
index 56ca3b6..4df4a7e 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -498,12 +498,14 @@ static int slic_card_download(struct adapter *adapter)
slic_reg32_write(&slic_regs->slic_wcs,
 baseaddress + codeaddr, FLUSH);
/* Write out instruction to low addr */
-   slic_reg32_write(&slic_regs->slic_wcs, instruction, 
FLUSH);
+   slic_reg32_write(&slic_regs->slic_wcs,
+   instruction, FLUSH);
instruction = *(u32 *)(fw->data + index);
index += 4;
 
/* Write out instruction to high addr */
-   slic_reg32_write(&slic_regs->slic_wcs, instruction, 
FLUSH);
+   slic_reg32_write(&slic_regs->slic_wcs,
+   instruction, FLUSH);
instruction = *(u32 *)(fw->data + index);
index += 4;
}
@@ -1533,14 +1535,18 @@ retry_rcvqfill:
dev_err(dev, "%s: LOW 32bits PHYSICAL ADDRESS 
== 0\n",
__func__);
dev_err(dev, "skb[%p] PROBLEM\n", skb);
-   dev_err(dev, " skbdata[%p]\n", 
skb->data);
+   dev_err(dev, " skbdata[%p]\n",
+   skb->data);
dev_err(dev, " skblen[%x]\n", skb->len);
dev_err(dev, " paddr[%p]\n", paddr);
dev_err(dev, " paddrl[%x]\n", paddrl);
dev_err(dev, " paddrh[%x]\n", paddrh);
-   dev_err(dev, " rcvq->head[%p]\n", 
rcvq->head);
-   dev_err(dev, " rcvq->tail[%p]\n", 
rcvq->tail);
-   dev_err(dev, " rcvq->count[%x]\n", 
rcvq->count);
+   dev_err(dev, " rcvq->head[%p]\n",
+   rcvq->head);
+   dev_err(dev, " rcvq->tail[%p]\n",
+   rcvq->tail);
+   dev_err(dev, " rcvq->count[%x]\n",
+   rcvq->count);
dev_err(dev, "SKIP THIS SKB\n");
goto retry_rcvqfill;
}
@@ -1549,14 +1555,18 @@ retry_rcvqfill:
dev_err(dev, "%s: LOW 32bits PHYSICAL ADDRESS 
== 0\n",
__func__);
dev_err(dev, "skb[%p] PROBLEM\n", skb);
-   dev_err(dev, " skbdata[%p]\n", 
skb->data);
+   dev_err(dev, " skbdata[%p]\n",
+   skb->data);
dev_err(dev, " skblen[%x]\n", skb->len);
dev_err(dev, " paddr[%p]\n", paddr);
dev_err(dev, " paddrl[%x]\n", paddrl);
dev_err(dev, " paddrh[%x]\n", paddrh);
-   dev_err(dev, " rcvq->head[%p]\n", 
rcvq->head);
-   dev_err(dev, " rcvq->tail[%p]\n", 
rcvq->tail);
-   dev_err(dev, " rcvq->count[%x]\n", 
rcvq->count);
+   dev_err(dev, " rcvq->head[%p]\n",
+   rcvq->head);
+   dev_err(dev, " rcvq->tail[%p]\n",
+   rcvq->tail);
+   dev_err(dev, " rcvq->count[%x]\n",
+   rcvq->count);
dev_err(dev, "GIVE TO CARD AN

[PATCH] Staging: slicoss: Fix long line issues in slicoss.c

[Sean Cleator]

A patch to the slicoss.c file to fix some of the long line issues found by the
checkpath.pl tool
 
Signed-off-by: Sean Cleator 

---
 drivers/staging/slicoss/slicoss.c | 39 ++-
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/slicoss/slicoss.c 
b/drivers/staging/slicoss/slicoss.c
index 56ca3b6..4df4a7e 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -498,12 +498,14 @@ static int slic_card_download(struct adapter *adapter)
slic_reg32_write(&slic_regs->slic_wcs,
 baseaddress + codeaddr, FLUSH);
/* Write out instruction to low addr */
-   slic_reg32_write(&slic_regs->slic_wcs, instruction, 
FLUSH);
+   slic_reg32_write(&slic_regs->slic_wcs,
+   instruction, FLUSH);
instruction = *(u32 *)(fw->data + index);
index += 4;
 
/* Write out instruction to high addr */
-   slic_reg32_write(&slic_regs->slic_wcs, instruction, 
FLUSH);
+   slic_reg32_write(&slic_regs->slic_wcs,
+   instruction, FLUSH);
instruction = *(u32 *)(fw->data + index);
index += 4;
}
@@ -1533,14 +1535,18 @@ retry_rcvqfill:
dev_err(dev, "%s: LOW 32bits PHYSICAL ADDRESS 
== 0\n",
__func__);
dev_err(dev, "skb[%p] PROBLEM\n", skb);
-   dev_err(dev, " skbdata[%p]\n", 
skb->data);
+   dev_err(dev, " skbdata[%p]\n",
+   skb->data);
dev_err(dev, " skblen[%x]\n", skb->len);
dev_err(dev, " paddr[%p]\n", paddr);
dev_err(dev, " paddrl[%x]\n", paddrl);
dev_err(dev, " paddrh[%x]\n", paddrh);
-   dev_err(dev, " rcvq->head[%p]\n", 
rcvq->head);
-   dev_err(dev, " rcvq->tail[%p]\n", 
rcvq->tail);
-   dev_err(dev, " rcvq->count[%x]\n", 
rcvq->count);
+   dev_err(dev, " rcvq->head[%p]\n",
+   rcvq->head);
+   dev_err(dev, " rcvq->tail[%p]\n",
+   rcvq->tail);
+   dev_err(dev, " rcvq->count[%x]\n",
+   rcvq->count);
dev_err(dev, "SKIP THIS SKB\n");
goto retry_rcvqfill;
}
@@ -1549,14 +1555,18 @@ retry_rcvqfill:
dev_err(dev, "%s: LOW 32bits PHYSICAL ADDRESS 
== 0\n",
__func__);
dev_err(dev, "skb[%p] PROBLEM\n", skb);
-   dev_err(dev, " skbdata[%p]\n", 
skb->data);
+   dev_err(dev, " skbdata[%p]\n",
+   skb->data);
dev_err(dev, " skblen[%x]\n", skb->len);
dev_err(dev, " paddr[%p]\n", paddr);
dev_err(dev, " paddrl[%x]\n", paddrl);
dev_err(dev, " paddrh[%x]\n", paddrh);
-   dev_err(dev, " rcvq->head[%p]\n", 
rcvq->head);
-   dev_err(dev, " rcvq->tail[%p]\n", 
rcvq->tail);
-   dev_err(dev, " rcvq->count[%x]\n", 
rcvq->count);
+   dev_err(dev, " rcvq->head[%p]\n",
+   rcvq->head);
+   dev_err(dev, " rcvq->tail[%p]\n",
+   rcvq->tail);
+   dev_err(dev, " rcvq->count[%x]\n",
+   rcvq->count);
dev_err(dev, "GIVE TO CARD ANYWAY\

[PATCH] Staging: dgnc: Fix long line coding style issues in dgnc_cls.h

[Sean Cleator]

A patch to fix the rest of the long line warnings in the dgnc_cls.h file
found by the checkpatch.pl tool

Signed-off-by: Sean Cleator 

---
 drivers/staging/dgnc/dgnc_cls.h | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_cls.h b/drivers/staging/dgnc/dgnc_cls.h
index 465d79a..32034e4 100644
--- a/drivers/staging/dgnc/dgnc_cls.h
+++ b/drivers/staging/dgnc/dgnc_cls.h
@@ -38,7 +38,10 @@
 struct cls_uart_struct {
u8 txrx;/* WR  RHR/THR - Holding Reg */
u8 ier; /* WR  IER - Interrupt Enable Reg */
-   u8 isr_fcr; /* WR  ISR/FCR - Interrupt Status Reg/Fifo 
Control Reg */
+   u8 isr_fcr; /*
+* WR  ISR/FCR - Interrupt Status Reg/Fifo
+* Control Reg
+*/
u8 lcr; /* WR  LCR - Line Control Reg */
u8 mcr; /* WR  MCR - Modem Control Reg */
u8 lsr; /* WR  LSR - Line Status Reg */
@@ -61,8 +64,11 @@ struct cls_uart_struct {
 #define UART_16654_FCR_RXTRIGGER_560x80
 #define UART_16654_FCR_RXTRIGGER_60 0xC0
 
-#define UART_IIR_CTSRTS0x20/* Received CTS/RTS 
change of state */
-#define UART_IIR_RDI_TIMEOUT   0x0C/* Receiver data TIMEOUT */
+#define UART_IIR_CTSRTS0x20/*
+*  Received CTS/RTS change of
+*  state
+*/
+#define UART_IIR_RDI_TIMEOUT   0x0C/* Receiver data TIMEOUT */
 
 /*
  * These are the EXTENDED definitions for the Exar 654's Interrupt
@@ -74,8 +80,14 @@ struct cls_uart_struct {
 #define UART_EXAR654_EFR_RTSDTR   0x40/* Auto RTS/DTR Flow Control Enable 
*/
 #define UART_EXAR654_EFR_CTSDSR   0x80/* Auto CTS/DSR Flow COntrol Enable 
*/
 
-#define UART_EXAR654_XOFF_DETECT  0x1 /* Indicates whether chip saw an 
incoming XOFF char  */
-#define UART_EXAR654_XON_DETECT   0x2 /* Indicates whether chip saw an 
incoming XON char */
+#define UART_EXAR654_XOFF_DETECT  0x1 /*
+  * Indicates whether chip saw an
+  * incoming XOFF char
+  */
+#define UART_EXAR654_XON_DETECT   0x2 /*
+  * Indicates whether chip saw an
+  * incoming XON char
+  */
 
 #define UART_EXAR654_IER_XOFF 0x20/* Xoff Interrupt Enable */
 #define UART_EXAR654_IER_RTSDTR   0x40/* Output Interrupt Enable */
-- 
2.1.3

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

[PATCH v2] Staging: dgnc: Fix long line coding style issues in dgnc_cls.h

[Sean Cleator]

A patch to fix the rest of the long line warnings in the dgnc_cls.h file
found by the checkpatch.pl tool

Signed-off-by: Sean Cleator 
---
 drivers/staging/dgnc/dgnc_cls.h | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_cls.h b/drivers/staging/dgnc/dgnc_cls.h
index 465d79a..ee7814b 100644
--- a/drivers/staging/dgnc/dgnc_cls.h
+++ b/drivers/staging/dgnc/dgnc_cls.h
@@ -36,9 +36,12 @@
  /
 
 struct cls_uart_struct {
-   u8 txrx;/* WR  RHR/THR - Holding Reg */
+   u8 txrx;/* WR  RHR/THR - Holding Reg */
u8 ier; /* WR  IER - Interrupt Enable Reg */
-   u8 isr_fcr; /* WR  ISR/FCR - Interrupt Status Reg/Fifo 
Control Reg */
+   u8 isr_fcr; /*
+* WR  ISR/FCR - Interrupt Status Reg/Fifo
+* Control Reg
+*/
u8 lcr; /* WR  LCR - Line Control Reg */
u8 mcr; /* WR  MCR - Modem Control Reg */
u8 lsr; /* WR  LSR - Line Status Reg */
@@ -61,8 +64,9 @@ struct cls_uart_struct {
 #define UART_16654_FCR_RXTRIGGER_560x80
 #define UART_16654_FCR_RXTRIGGER_60 0xC0
 
-#define UART_IIR_CTSRTS0x20/* Received CTS/RTS 
change of state */
-#define UART_IIR_RDI_TIMEOUT   0x0C/* Receiver data TIMEOUT */
+/* Received CTS/RTS change of state */
+#define UART_IIR_CTSRTS0x20
+#define UART_IIR_RDI_TIMEOUT   0x0C/* Receiver data TIMEOUT */
 
 /*
  * These are the EXTENDED definitions for the Exar 654's Interrupt
@@ -74,8 +78,10 @@ struct cls_uart_struct {
 #define UART_EXAR654_EFR_RTSDTR   0x40/* Auto RTS/DTR Flow Control Enable 
*/
 #define UART_EXAR654_EFR_CTSDSR   0x80/* Auto CTS/DSR Flow COntrol Enable 
*/
 
-#define UART_EXAR654_XOFF_DETECT  0x1 /* Indicates whether chip saw an 
incoming XOFF char  */
-#define UART_EXAR654_XON_DETECT   0x2 /* Indicates whether chip saw an 
incoming XON char */
+/* Indicates whether chip saw an incoming XOFF char */
+#define UART_EXAR654_XOFF_DETECT  0x1
+/* Indicates whether chip saw an incoming XON char */
+#define UART_EXAR654_XON_DETECT   0x2
 
 #define UART_EXAR654_IER_XOFF 0x20/* Xoff Interrupt Enable */
 #define UART_EXAR654_IER_RTSDTR   0x40/* Output Interrupt Enable */
-- 
2.1.3

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

Re: [Outreachy kernel] Re: [PATCH] Staging: ccree: Don't use volatile for monitor_lock

[Sean Paul]

On Mon, Sep 11, 2017 at 12:08 PM, Srishti Sharma  wrote:
> On Mon, Sep 11, 2017 at 9:34 PM, Greg KH  wrote:
>> On Mon, Sep 11, 2017 at 09:29:31PM +0530, Srishti Sharma wrote:
>>> The use of volatile for the variable monitor_lock is unnecessary.
>>>
>>> Signed-off-by: Srishti Sharma 
>>> ---
>>>  drivers/staging/ccree/ssi_request_mgr.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/ccree/ssi_request_mgr.c 
>>> b/drivers/staging/ccree/ssi_request_mgr.c
>>> index e5c2f92..7d77941 100644
>>> --- a/drivers/staging/ccree/ssi_request_mgr.c
>>> +++ b/drivers/staging/ccree/ssi_request_mgr.c
>>> @@ -49,7 +49,7 @@ struct ssi_request_mgr_handle {
>>>   dma_addr_t dummy_comp_buff_dma;
>>>   struct cc_hw_desc monitor_desc;
>>>
>>> - volatile unsigned long monitor_lock;
>>> + unsigned long monitor_lock;
>>
>> While volatile is not right, odds are, this is still totally wrong as
>> well.  How about using a "real" lock instead?
>
> I tried to find where is this variable being used in the code, but I
> didn't find any usage of it . It might be an important attribute of
> this structure definition but, I don't see it's value being set to
> anything or being used somewhere .
>

AFAICT, it's not used. Your patch should just remove it instead :)

Sean

> Regards,
> Srishti
>>
>> thanks,
>>
>> greg k-h
>
> --
> 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/CAB3L5oxcyhgyy8EuGuPo9QtJQd-W7JTgQQE1PfopZFmSx58P9g%40mail.gmail.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] Staging: ccree: Remove unused variable monitor_lock

[Sean Paul]

On Mon, Sep 11, 2017 at 12:28 PM, Srishti Sharma  wrote:
> Remove the variable monitor_lock as it is not used anywhere.
>
> Signed-off-by: Srishti Sharma 

Reviewed-by: Sean Paul 

> ---
> Changes in v2:
>  -The variable that was not to be declared as volatile can be
>   eliminated as it is not being used anywhere.
>
>  drivers/staging/ccree/ssi_request_mgr.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/ccree/ssi_request_mgr.c 
> b/drivers/staging/ccree/ssi_request_mgr.c
> index e5c2f92..b94a91f 100644
> --- a/drivers/staging/ccree/ssi_request_mgr.c
> +++ b/drivers/staging/ccree/ssi_request_mgr.c
> @@ -49,7 +49,6 @@ struct ssi_request_mgr_handle {
> dma_addr_t dummy_comp_buff_dma;
> struct cc_hw_desc monitor_desc;
>
> -   volatile unsigned long monitor_lock;
>  #ifdef COMP_IN_WQ
> struct workqueue_struct *workq;
> struct delayed_work compwork;
> --
> 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/1505147317-13411-1-git-send-email-srishtishar%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 Resend] staging: media: lirc: style fix - replace hard-coded function names

[Sean Young]

On Tue, Nov 28, 2017 at 06:47:08PM +0100, Martin Homuth wrote:
> This patch fixes the remaining coding style warnings in the lirc module.
> Instead of hard coding the function name the __func__ variable
> should be used.
> 
> It fixes the following checkpatch.pl warning:
> 
> WARNING: Prefer using '"%s...", __func__' to using 'read', this
> function's name, in a string
> 
> Signed-off-by: Martin Homuth 
> ---
>  drivers/staging/media/lirc/lirc_zilog.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_zilog.c
> b/drivers/staging/media/lirc/lirc_zilog.c
> index 6bd0717bf76e..be68ee652071 100644
> --- a/drivers/staging/media/lirc/lirc_zilog.c
> +++ b/drivers/staging/media/lirc/lirc_zilog.c

I'm afraid that lirc_zilog has been re-written.

https://patchwork.linuxtv.org/patch/45189/

It hasn't been merged yet, but I suspect that is imminent.


Sean

> @@ -888,9 +888,9 @@ static ssize_t read(struct file *filep, char __user
> *outbuf, size_t n,
>   unsigned int m;
>   DECLARE_WAITQUEUE(wait, current);
> 
> - dev_dbg(ir->dev, "read called\n");
> + dev_dbg(ir->dev, "%s called\n", __func__);
>   if (n % rbuf->chunk_size) {
> - dev_dbg(ir->dev, "read result = -EINVAL\n");
> + dev_dbg(ir->dev, "%s result = -EINVAL\n", __func__);
>   return -EINVAL;
>   }
> 
> @@ -949,7 +949,7 @@ static ssize_t read(struct file *filep, char __user
> *outbuf, size_t n,
>   retries++;
>   }
>   if (retries >= 5) {
> - dev_err(ir->dev, "Buffer read failed!\n");
> + dev_err(ir->dev, "%s failed!\n", __func__);
>   ret = -EIO;
>   }
>   }
> @@ -959,7 +959,7 @@ static ssize_t read(struct file *filep, char __user
> *outbuf, size_t n,
>   put_ir_rx(rx, false);
>   set_current_state(TASK_RUNNING);
> 
> - dev_dbg(ir->dev, "read result = %d (%s)\n", ret,
> + dev_dbg(ir->dev, "%s result = %d (%s)\n", __func__, ret,
>   ret ? "Error" : "OK");
> 
>   return ret ? ret : written;
> -- 
> 2.13.6
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] gpio: mediatek: add driver for MT7621

[Sean Wang]

Hi,

On Fri, 2018-06-08 at 13:59 +0200, Linus Walleij wrote:
> Hi Sergio!
> 
> Thanks for your patch!
> 
> Given that we have combined pin control and GPIO drivers for
> almost all Mediatek chips in drivers/pinctrl/mediatek/*
> I would ideally like to have some input from the Mediatek
> maintainers (especially Sean Wang) on this, especially:
> 
> - Is MT7621 a non-pincontrol GPIO controller, or can it
>   eventually use pin control as a back-end? Will a separate
>   pin control driver appear later for this SoC?
> 

MT7621 also have the circuit for pad setup tweaking Tx driving and mux
setup switching to either gpio mode or specific hardware mode, but the
circuit for all of them is being accessed in a different register range
from gpio controller being implemented here.

the part of pad or mux control for MT7621 I thought that is really
simple, so it seems worth joining pinmux and pinconf together into a
single driver to become a full function about pin setup. 

> - Would it make sense to have a combined driver just like
>   for the other Mediatek SoCs in drivers/pinctrl/mediatek?
>   If this GPIO controller does not do pin control I understand
>   why it is submitted as a GPIO driver only.
> 
> drivers/pinctrl/mediatek/pinctrl-mt7622.c is suspiciously
> similarly named. Is this a relative or just as different as
> night and day?
> 

The MT7621 is just as completely different as night and day from MT7622.

MT7622 pinctrl originate from MediaTek IPs but MT7621 pinctrl originate
from Ralink IPs and even MT7621 should be the last one machine using the
Ralink pinctrl IPs. And for these machine MT762x appearing later MT7622,
they all will be developed based on pinctrl-mt7622 architecture .

> Also you can see that this driver has a built-in GPIO driver,
> using an external interrupt.
> 
> On Sat, Jun 2, 2018 at 9:30 AM, Sergio Paracuellos

[ ... ]

> I guess you want to use
> builtin_platform_driver()?
> 
> Yours,
> Linus Walleij


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

r8192e_pci driver broken 3.14+

[Sean MacLennan]

Commit 198e0d17c on November 2 by Rashika Kheria breaks the r8192e_pci
driver on my laptop. The bulk of the commit is fine. It is just the
following change that causes grief:

diff --git a/drivers/staging/rtl8192e/rtllib_tx.c 
b/drivers/staging/rtl8192e/rtllib_tx.c
index 3183627..7796488 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -171,7 +171,7 @@ inline int rtllib_put_snap(u8 *data, u16 h_proto)
snap->oui[1] = oui[1];
snap->oui[2] = oui[2];

-   *(u16 *)(data + SNAP_SIZE) = htons(h_proto);
+   *(u16 *)(data + SNAP_SIZE) = h_proto;

return SNAP_SIZE + sizeof(u16);
 }

I am not sure what platform this was tested on, but I believe for any
little endian machine the htons() is required. Reverting this part of
the commit gets me a working driver. I sent this email using it ;)

Sorry it took so long to report, I don't really use the wireless in
the winter.

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

Re: r8192e_pci driver broken 3.14+

[Sean MacLennan]

A sparse error fixup removed a htons() which is required for the driver
to function. Put the htons() back.

Signed-off-by: Sean MacLennan 
---
diff --git a/drivers/staging/rtl8192e/rtllib_tx.c 
b/drivers/staging/rtl8192e/rtllib_tx.c
index 11d0a9d..3e79bff 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -171,7 +171,7 @@ inline int rtllib_put_snap(u8 *data, u16 h_proto)
snap->oui[1] = oui[1];
snap->oui[2] = oui[2];
 
-   *(u16 *)(data + SNAP_SIZE) = h_proto;
+   *(u16 *)(data + SNAP_SIZE) = htons(h_proto);
 
return SNAP_SIZE + sizeof(u16);
 }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: r8192e_pci driver broken 3.14+

[Sean MacLennan]

Fix a sparse error in the htons() call. htons() returns a __be16 not a
u16.

Signed-off-by: Sean MacLennan 
---
diff --git a/drivers/staging/rtl8192e/rtllib_tx.c 
b/drivers/staging/rtl8192e/rtllib_tx.c
index 3e79bff..b7dd153 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -171,7 +171,7 @@ inline int rtllib_put_snap(u8 *data, u16 h_proto)
snap->oui[1] = oui[1];
snap->oui[2] = oui[2];
 
-   *(u16 *)(data + SNAP_SIZE) = htons(h_proto);
+   *(__be16 *)(data + SNAP_SIZE) = htons(h_proto);
 
return SNAP_SIZE + sizeof(u16);
 }

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

Re: r8192e_pci driver broken 3.14+

[Sean MacLennan]

On Sat, 19 Apr 2014 11:17:23 -0700
Greg KH  wrote:

> On Sat, Apr 19, 2014 at 02:12:34PM -0400, Sean MacLennan wrote:
> > Fix a sparse error in the htons() call. htons() returns a __be16
> > not a u16.
> > 
> > Signed-off-by: Sean MacLennan 
> > ---
> 
> You sent 2 patches, which should I apply?
> 
> I've deleted both from my inbox, please resend the proper one, with a
> subject: that reflects that it is a patch.

They where two different patches. Would you prefer one patch? I split
it in two since the first one is needed. The second fixes the sparse
error.

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

[PATCH] r8192e_pci driver broken 3.14+

[Sean MacLennan]

A sparse error fixup removed a htons() which is required for the driver
to function. This patch puts the htons() back and fixes the sparse
warning correctly by changing the left side cast.

Signed-off-by: Sean MacLennan 
---
diff --git a/drivers/staging/rtl8192e/rtllib_tx.c 
b/drivers/staging/rtl8192e/rtllib_tx.c
index 11d0a9d..b7dd153 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -171,7 +171,7 @@ inline int rtllib_put_snap(u8 *data, u16 h_proto)
snap->oui[1] = oui[1];
snap->oui[2] = oui[2];
 
-   *(u16 *)(data + SNAP_SIZE) = h_proto;
+   *(__be16 *)(data + SNAP_SIZE) = htons(h_proto);
 
return SNAP_SIZE + sizeof(u16);
 }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [[media] rc] e662671619: BUG: kernel hang in test stage

[Sean Young]

On Mon, May 08, 2017 at 08:13:37PM +0800, kernel test robot wrote:
> Greetings,
> 
> 0day kernel testing robot got the below dmesg and the first bad commit is
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> 
> commit e66267161971155a8b4756b4e17f2f2f82b9f842
> Author: Sean Young 
> AuthorDate: Tue Mar 7 17:07:59 2017 -0300
> Commit: Mauro Carvalho Chehab 
> CommitDate: Wed Apr 5 14:50:57 2017 -0300
> 
> [media] rc: promote lirc_sir out of staging
> 
> Rename lirc_sir to sir_ir in the process.
> 
> Signed-off-by: Sean Young 
> Signed-off-by: Mauro Carvalho Chehab 

Here the sir_ir gets in an infinite loop in its interrupt handler, since the
hardware is different from what it expects.

I was looking in all the wrong places so it took too long to find this. :/

I'll send a patch as a reply to this email.

Thanks,

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

[PATCH] [media] sir_ir: infinite loop in interrupt handler

[Sean Young]

Since this driver does no detection of hardware, it might be used with
a non-sir port. Escape out if we are spinning.

Signed-off-by: Sean Young 
---
 drivers/media/rc/sir_ir.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/rc/sir_ir.c b/drivers/media/rc/sir_ir.c
index e12ec50..90a5f8f 100644
--- a/drivers/media/rc/sir_ir.c
+++ b/drivers/media/rc/sir_ir.c
@@ -183,9 +183,15 @@ static irqreturn_t sir_interrupt(int irq, void *dev_id)
static unsigned long delt;
unsigned long deltintr;
unsigned long flags;
+   int counter = 0;
int iir, lsr;
 
while ((iir = inb(io + UART_IIR) & UART_IIR_ID)) {
+   if (++counter > 256) {
+   dev_err(&sir_ir_dev->dev, "Trapped in interrupt");
+   break;
+   }
+
switch (iir & UART_IIR_ID) { /* FIXME toto treba preriedit */
case UART_IIR_MSI:
(void)inb(io + UART_MSR);
-- 
2.9.4

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

Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging

[Sean Paul]

t I noticed there are a few functions
declared/defined in the osindependent code that are never used outside of it, so
we have dead code off the hop.

> 
> Please be clear, I am not trying to dictate to anyone.  The code is GPL,
> it is fine to include it with whatever modifications you deem
> appropriate.  Since individual distributions are already doing that, it
> will still simplify our life somewhat if it is in the upstream kernel,
> in whatever form, rather than in multiple forks.
> 
> > Remember, this code needs us, we don't need this code at all :)
> 
> I assume that that was not meant that way, but that came over as
> slightly unfriendly to me.  I am assuming here that we are looking for
> the best way to do something which will be useful to a lot of people.
> 

IMO, in order to accept the driver in drm, the osindependent code needs to be
stripped out and it needs to be converted to atomic. Whether you want to do
this out of tree, or in staging is up to you. As Dave mentioned, people often
overlook staging when making drm core changes, so you'll likely face the same
moving target issues either way.

Sean


> Regards
> Michael
> 
> > good luck!
> > 
> > greg k-h
> > 
> -- 
> Michael Thayer | VirtualBox engineer
> ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt
> 
> ORACLE Deutschland B.V. & Co. KG
> Hauptverwaltung: Riesstraße 25, D-80992 München
> Registergericht: Amtsgericht München, HRA 95603
> 
> Komplementärin: ORACLE Deutschland Verwaltung B.V.
> Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister
> der Handelskammer Midden-Nederland, Nr. 30143697
> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: vboxvideo: Add vboxvideo to drivers/staging

[Sean Paul]

On Wed, Jun 14, 2017 at 11:34:10AM +0200, Michael Thayer wrote:
> Hello Sean,
> 
> 13.06.2017 20:03, Sean Paul wrote:
> [Discussion of vboxvideo driver clean-up.]
> 
> > First, thank you for your submission.
> 
> Thank you for your feedback.
> 
> [Discussion of the OS-independent code in the driver submission.]
> 
> > I took a quick skim through your driver, and there doesn't seem to be much
> > secret sauce there that will be tough to keep up-to-date across platforms.
> 
> I have two particular concerns there: first if we add new functionality
> (which we would do out of tree first) it will need porting over.
> Acceleration support is the most likely candidate.  And if someone does
> make fixes to that part of the code in the kernel tree they will also
> need porting over.  I agree, that concern is probably overblown, and
> best addressed by keeping that part of the code as close to our tree as
> possible while still meeting kernel standards (hence my question as to
> what that would be).

Once the code is upstream, it's going to be difficult to motivate developers to
keep the driver close to your downstream version. If I'm working on feature X
which touches your driver, I'm not going to figure out how your internal version
works so that I might ease your pain. I don't mean that to be rude, just
realistic.

Sean

> 
> The second concern is not relevant to DRI, but it concerns our other
> guest drivers (not the host one with the C++ in it Greg!) which Hans
> also expressed interest in putting upstream after seeing how vboxvideo
> fares.  The OS-independent part is quite a bit larger and more volatile,
> though it has thankfully stablised a lot.  That concern is probably also
> overblown, though I do wonder whether upstreaming those driver is the
> best solution (that would be Hans's call though).
> 
> > One other concern I have is that I noticed there are a few functions
> > declared/defined in the osindependent code that are never used outside of 
> > it, so
> > we have dead code off the hop.
> 
> If the OS-independent part gets converted then they would be removed in
> the process.  Thank you for the reminder.
> 
> [...]
> 
> > IMO, in order to accept the driver in drm, the osindependent code needs to 
> > be
> > stripped out and it needs to be converted to atomic. Whether you want to do
> > this out of tree, or in staging is up to you. As Dave mentioned, people 
> > often
> > overlook staging when making drm core changes, so you'll likely face the 
> > same
> > moving target issues either way.
> 
> Conversion to Atomic would probably have to happen at some time or
> another anyway.  I have put that off (out-of-tree) so far because I was
> tracking the AST driver as closely as possible as the simplest way of
> picking up fixes, and because Dave, who wrote that, knows much more
> about drm drivers than me.
> 
> [...]
> 
> Regards
> Michael
> -- 
> Michael Thayer | VirtualBox engineer
> ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt
> 
> ORACLE Deutschland B.V. & Co. KG
> Hauptverwaltung: Riesstraße 25, D-80992 München
> Registergericht: Amtsgericht München, HRA 95603
> 
> Komplementärin: ORACLE Deutschland Verwaltung B.V.
> Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister
> der Handelskammer Midden-Nederland, Nr. 30143697
> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging:rtl8192e: Usage count off by one

[Sean MacLennan]

The rtllib driver is not calling try_module_get() when loading the
encryption modules. Because of this, you can never remove the module
once you have used it one (i.e. bring up the wireless interface).

Signed-off-by: Sean MacLennan 
---
 drivers/staging/rtl8192e/rtllib_softmac.c | 2 +-
 drivers/staging/rtl8192e/rtllib_wx.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c 
b/drivers/staging/rtl8192e/rtllib_softmac.c
index d0fedb0..acccf9b 100644
--- a/drivers/staging/rtl8192e/rtllib_softmac.c
+++ b/drivers/staging/rtl8192e/rtllib_softmac.c
@@ -3328,7 +3328,7 @@ static int rtllib_wpa_set_encryption(struct rtllib_device 
*ieee,
goto done;
}
new_crypt->ops = ops;
-   if (new_crypt->ops)
+   if (new_crypt->ops && try_module_get(new_crypt->ops->owner))
new_crypt->priv =
new_crypt->ops->init(param->u.crypt.idx);
 
diff --git a/drivers/staging/rtl8192e/rtllib_wx.c 
b/drivers/staging/rtl8192e/rtllib_wx.c
index 80f7a09..84e6272 100644
--- a/drivers/staging/rtl8192e/rtllib_wx.c
+++ b/drivers/staging/rtl8192e/rtllib_wx.c
@@ -623,7 +623,7 @@ int rtllib_wx_set_encode_ext(struct rtllib_device *ieee,
goto done;
}
new_crypt->ops = ops;
-   if (new_crypt->ops)
+   if (new_crypt->ops && try_module_get(new_crypt->ops->owner))
new_crypt->priv = new_crypt->ops->init(idx);
 
if (new_crypt->priv == NULL) {
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: media: lirc: style fix, using octal file permissions

[Sean Young]

On Sat, Jan 07, 2017 at 04:02:55PM +1300, Derek Robson wrote:
> Change file permissions to octal style.
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/media/lirc/lirc_imon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/lirc/lirc_imon.c 
> b/drivers/staging/media/lirc/lirc_imon.c
> index 1e650fba4a92..6c8a4a15278e 100644
> --- a/drivers/staging/media/lirc/lirc_imon.c
> +++ b/drivers/staging/media/lirc/lirc_imon.c
> @@ -182,7 +182,7 @@ MODULE_DESCRIPTION(MOD_DESC);
>  MODULE_VERSION(MOD_VERSION);
>  MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(usb, imon_usb_id_table);
> -module_param(debug, int, S_IRUGO | S_IWUSR);
> +module_param(debug, int, 0644);
>  MODULE_PARM_DESC(debug, "Debug messages: 0=no, 1=yes(default: no)");
>  
>  static void free_imon_context(struct imon_context *context)

In the current media tree, drivers/staging/media/lirc/lirc_imon.c has
been merged with drivers/media/rc/imon.c already, I'm afraid. This
patch no longer applies.


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

Re: [PATCH RESEND] staging: media: lirc: use new parport device model

[Sean Young]

On Sat, Jan 21, 2017 at 12:55:54AM +, Sudip Mukherjee wrote:
> From: Sudip Mukherjee 
> 
> Modify lirc_parallel driver to use the new parallel port device model.
> 
> Signed-off-by: Sudip Mukherjee 
> ---
> 
> Resending after more than one year.
> Prevoius patch is at https://patchwork.kernel.org/patch/7883591/

Since noone ported lirc_parallel to rc-core, the lirc_parallel staging
driver has been droppped from the current media tree.

I have ported a few other lirc drivers to rc-core but I never found
anyone using lirc_parallel or the hardware itself.


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

[PATCH] Staging: Octeon USB: Fix space prohibited between function name and open parenthesis

[Sean Darcy]

This patch fixes the following checkpatch.pl warning in 
drivers/staging/octeon-usb/octeon-hcd.c 
WARNING: space prohibited between function name and open parenthesis '(' 

Signed-off-by: Sean Darcy 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index 1daeb31..0408a12 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -412,7 +412,7 @@ struct octeon_hcd {
type c; \
while (1) { \
c.u32 = __cvmx_usb_read_csr32(usb, address);\
-   if (c.s.field op (value)) { \
+   if (c.s.field op(value)) {  \
result = 0; \
break;  \
} else if (cvmx_get_cycle() > done) {   \
-- 
1.9.1

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

Re: [PATCH v3 28/28] drm: vboxvideo: switch to drm_*_get(), drm_*_put() helpers

[Sean Paul]

On Fri, Aug 11, 2017 at 03:26:45PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 11-08-17 14:33, Cihangir Akturk wrote:
> > Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference()
> > and drm_*_unreference() helpers.
> > 
> > drm_*_reference() and drm_*_unreference() functions are just
> > compatibility alias for drm_*_get() and drm_*_put() and should not be
> > used by new code. So convert all users of compatibility functions to
> > use the new APIs.
> > 
> > Generated by: scripts/coccinelle/api/drm-get-put.cocci
> > 
> > Signed-off-by: Cihangir Akturk 
> 
> Thank you for doing this, looks good to me:
> 
> Reviewed-by: Hans de Goede 
> 

Applied to drm-misc-next, thank you for the review!

Sean

> Regards,
> 
> Hans
> 
> 
> 
> > ---
> >   drivers/staging/vboxvideo/vbox_fb.c   | 2 +-
> >   drivers/staging/vboxvideo/vbox_main.c | 8 
> >   drivers/staging/vboxvideo/vbox_mode.c | 2 +-
> >   3 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/staging/vboxvideo/vbox_fb.c 
> > b/drivers/staging/vboxvideo/vbox_fb.c
> > index bf66358..c157284 100644
> > --- a/drivers/staging/vboxvideo/vbox_fb.c
> > +++ b/drivers/staging/vboxvideo/vbox_fb.c
> > @@ -343,7 +343,7 @@ void vbox_fbdev_fini(struct drm_device *dev)
> > vbox_bo_unpin(bo);
> > vbox_bo_unreserve(bo);
> > }
> > -   drm_gem_object_unreference_unlocked(afb->obj);
> > +   drm_gem_object_put_unlocked(afb->obj);
> > afb->obj = NULL;
> > }
> > drm_fb_helper_fini(&fbdev->helper);
> > diff --git a/drivers/staging/vboxvideo/vbox_main.c 
> > b/drivers/staging/vboxvideo/vbox_main.c
> > index d0c6ec7..80bd039 100644
> > --- a/drivers/staging/vboxvideo/vbox_main.c
> > +++ b/drivers/staging/vboxvideo/vbox_main.c
> > @@ -40,7 +40,7 @@ static void vbox_user_framebuffer_destroy(struct 
> > drm_framebuffer *fb)
> > struct vbox_framebuffer *vbox_fb = to_vbox_framebuffer(fb);
> > if (vbox_fb->obj)
> > -   drm_gem_object_unreference_unlocked(vbox_fb->obj);
> > +   drm_gem_object_put_unlocked(vbox_fb->obj);
> > drm_framebuffer_cleanup(fb);
> > kfree(fb);
> > @@ -198,7 +198,7 @@ static struct drm_framebuffer 
> > *vbox_user_framebuffer_create(
> >   err_free_vbox_fb:
> > kfree(vbox_fb);
> >   err_unref_obj:
> > -   drm_gem_object_unreference_unlocked(obj);
> > +   drm_gem_object_put_unlocked(obj);
> > return ERR_PTR(ret);
> >   }
> > @@ -472,7 +472,7 @@ int vbox_dumb_create(struct drm_file *file,
> > return ret;
> > ret = drm_gem_handle_create(file, gobj, &handle);
> > -   drm_gem_object_unreference_unlocked(gobj);
> > +   drm_gem_object_put_unlocked(gobj);
> > if (ret)
> > return ret;
> > @@ -525,7 +525,7 @@ vbox_dumb_mmap_offset(struct drm_file *file,
> > bo = gem_to_vbox_bo(obj);
> > *offset = vbox_bo_mmap_offset(bo);
> > -   drm_gem_object_unreference(obj);
> > +   drm_gem_object_put(obj);
> > ret = 0;
> >   out_unlock:
> > diff --git a/drivers/staging/vboxvideo/vbox_mode.c 
> > b/drivers/staging/vboxvideo/vbox_mode.c
> > index 996da1c..e5b6383 100644
> > --- a/drivers/staging/vboxvideo/vbox_mode.c
> > +++ b/drivers/staging/vboxvideo/vbox_mode.c
> > @@ -812,7 +812,7 @@ static int vbox_cursor_set2(struct drm_crtc *crtc, 
> > struct drm_file *file_priv,
> >   out_unreserve_bo:
> > vbox_bo_unreserve(bo);
> >   out_unref_obj:
> > -   drm_gem_object_unreference_unlocked(obj);
> > +   drm_gem_object_put_unlocked(obj);
> > return ret;
> >   }
> > 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 28/28] drm: vboxvideo: switch to drm_*_get(), drm_*_put() helpers

[Sean Paul]

On Fri, Aug 11, 2017 at 12:11 PM, Hans de Goede  wrote:
> Hi,
>
> On 11-08-17 18:04, Sean Paul wrote:
>>
>> On Fri, Aug 11, 2017 at 03:26:45PM +0200, Hans de Goede wrote:
>>>
>>> Hi,
>>>
>>> On 11-08-17 14:33, Cihangir Akturk wrote:
>>>>
>>>> Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference()
>>>> and drm_*_unreference() helpers.
>>>>
>>>> drm_*_reference() and drm_*_unreference() functions are just
>>>> compatibility alias for drm_*_get() and drm_*_put() and should not be
>>>> used by new code. So convert all users of compatibility functions to
>>>> use the new APIs.
>>>>
>>>> Generated by: scripts/coccinelle/api/drm-get-put.cocci
>>>>
>>>> Signed-off-by: Cihangir Akturk 
>>>
>>>
>>> Thank you for doing this, looks good to me:
>>>
>>> Reviewed-by: Hans de Goede 
>>>
>>
>> Applied to drm-misc-next, thank you for the review!
>
>
> Erm vboxvideo is in staging, does this mean all patches for
> it will now go through drm-misc-next despite it being in
> staging (*) ?  Because if some patches get merged through
> drm-misc-next and some through Greg's staging repo that
> is not going to end well.
>

Hi Hans,
Thanks for pointing this out. I picked it up as I was vacuuming up the
rest of the set that was not yet applied, not realizing the staging
implications. It's not my intention to start taking vboxvideo through
-misc.

Hopefully this won't cause any nasty conflicts with Greg's staging
tree, and we can treat it as a one-off. If it does cause problems, I
can revert it in -misc in favor of taking it through staging.

Sean

> Regards,
>
> Hans
>
>
>
> *) that is fine, the same is done for e.g. the media drivers afaik
>
>
>
>>
>> Sean
>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>> ---
>>>>drivers/staging/vboxvideo/vbox_fb.c   | 2 +-
>>>>drivers/staging/vboxvideo/vbox_main.c | 8 
>>>>drivers/staging/vboxvideo/vbox_mode.c | 2 +-
>>>>3 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/vboxvideo/vbox_fb.c
>>>> b/drivers/staging/vboxvideo/vbox_fb.c
>>>> index bf66358..c157284 100644
>>>> --- a/drivers/staging/vboxvideo/vbox_fb.c
>>>> +++ b/drivers/staging/vboxvideo/vbox_fb.c
>>>> @@ -343,7 +343,7 @@ void vbox_fbdev_fini(struct drm_device *dev)
>>>> vbox_bo_unpin(bo);
>>>> vbox_bo_unreserve(bo);
>>>> }
>>>> -   drm_gem_object_unreference_unlocked(afb->obj);
>>>> +   drm_gem_object_put_unlocked(afb->obj);
>>>> afb->obj = NULL;
>>>> }
>>>> drm_fb_helper_fini(&fbdev->helper);
>>>> diff --git a/drivers/staging/vboxvideo/vbox_main.c
>>>> b/drivers/staging/vboxvideo/vbox_main.c
>>>> index d0c6ec7..80bd039 100644
>>>> --- a/drivers/staging/vboxvideo/vbox_main.c
>>>> +++ b/drivers/staging/vboxvideo/vbox_main.c
>>>> @@ -40,7 +40,7 @@ static void vbox_user_framebuffer_destroy(struct
>>>> drm_framebuffer *fb)
>>>> struct vbox_framebuffer *vbox_fb = to_vbox_framebuffer(fb);
>>>> if (vbox_fb->obj)
>>>> -   drm_gem_object_unreference_unlocked(vbox_fb->obj);
>>>> +   drm_gem_object_put_unlocked(vbox_fb->obj);
>>>> drm_framebuffer_cleanup(fb);
>>>> kfree(fb);
>>>> @@ -198,7 +198,7 @@ static struct drm_framebuffer
>>>> *vbox_user_framebuffer_create(
>>>>err_free_vbox_fb:
>>>> kfree(vbox_fb);
>>>>err_unref_obj:
>>>> -   drm_gem_object_unreference_unlocked(obj);
>>>> +   drm_gem_object_put_unlocked(obj);
>>>> return ERR_PTR(ret);
>>>>}
>>>> @@ -472,7 +472,7 @@ int vbox_dumb_create(struct drm_file *file,
>>>> return ret;
>>>> ret = drm_gem_handle_create(file, gobj, &handle);
>>>> -   drm_gem_object_unreference_unlocked(gobj);
>>>> +   drm_gem_object_put_unlocked(gobj);
>>>> if (ret)
>>>> return ret;
>>>> @@ -525,7 +525,7 @@ vbox_dumb_mmap_offset(struct drm_file *

Re: [Outreachy kernel] [PATCH] Staging: media: imx: Prefer using BIT macro

[Sean Paul]

On Fri, Sep 8, 2017 at 11:11 AM, Srishti Sharma  wrote:
> Use BIT(x) instead of (1<
> Signed-off-by: Srishti Sharma 
> ---
>  drivers/staging/media/imx/imx-media.h | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-media.h 
> b/drivers/staging/media/imx/imx-media.h
> index d409170..e5b8d29 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -310,16 +310,16 @@ void imx_media_capture_device_set_format(struct 
> imx_media_video_dev *vdev,
>  void imx_media_capture_device_error(struct imx_media_video_dev *vdev);
>
>  /* subdev group ids */
> -#define IMX_MEDIA_GRP_ID_SENSOR(1 << 8)
> -#define IMX_MEDIA_GRP_ID_VIDMUX(1 << 9)
> -#define IMX_MEDIA_GRP_ID_CSI2  (1 << 10)
> +#define IMX_MEDIA_GRP_ID_SENSORBIT(8)
> +#define IMX_MEDIA_GRP_ID_VIDMUXBIT(9)
> +#define IMX_MEDIA_GRP_ID_CSI2  BIT(10)
>  #define IMX_MEDIA_GRP_ID_CSI_BIT   11
>  #define IMX_MEDIA_GRP_ID_CSI   (0x3 << IMX_MEDIA_GRP_ID_CSI_BIT)
> -#define IMX_MEDIA_GRP_ID_CSI0  (1 << IMX_MEDIA_GRP_ID_CSI_BIT)
> +#define IMX_MEDIA_GRP_ID_CSI0  BIT(IMX_MEDIA_GRP_ID_CSI_BIT)
>  #define IMX_MEDIA_GRP_ID_CSI1  (2 << IMX_MEDIA_GRP_ID_CSI_BIT)
> -#define IMX_MEDIA_GRP_ID_VDIC  (1 << 13)
> -#define IMX_MEDIA_GRP_ID_IC_PRP(1 << 14)
> -#define IMX_MEDIA_GRP_ID_IC_PRPENC (1 << 15)
> -#define IMX_MEDIA_GRP_ID_IC_PRPVF  (1 << 16)
> +#define IMX_MEDIA_GRP_ID_VDIC  BIT(13)
> +#define IMX_MEDIA_GRP_ID_IC_PRPBIT(14)
> +#define IMX_MEDIA_GRP_ID_IC_PRPENC BIT(15)
> +#define IMX_MEDIA_GRP_ID_IC_PRPVF  BIT(16)

Hi Srishti,
Thanks for your patch.

Perhaps this is just personal preference, but I find the previous
version more readable. Since IMX_MEDIA_GRP_ID_CSI and
IMX_MEDIA_GRP_ID_CSI1 are multi-bit fields, you can't fully eliminate
the bit shift operations, so you end up with a mix, which is kind of
ugly.

Sean

>
>  #endif
> --
> 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/1504883469-8127-1-git-send-email-srishtishar%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 RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems

[Sean Paul]

On Thu, Jan 2, 2014 at 4:27 PM, Russell King
 wrote:
> Subsystems such as ALSA, DRM and others require a single card-level
> device structure to represent a subsystem.  However, firmware tends to
> describe the individual devices and the connections between them.
>
> Therefore, we need a way to gather up the individual component devices
> together, and indicate when we have all the component devices.
>
> We do this in DT by providing a "superdevice" node which specifies
> the components, eg:
>
> imx-drm {
> compatible = "fsl,drm";
> crtcs = <&ipu1>;
> connectors = <&hdmi>;
> };
>

Hi Russell,
This looks really good. I'd definitely like to use it for the exynos
drm driver both to bind the various IP blocks together, and also to
pull in any attached bridges that are specified in the device tree.
Along those lines, it might be worthwhile to pull some of the master
bind functionality in your next patch into drm helpers so other
drivers can use them, and so we have concrete bindings across drm.
Make sense?

Sean



> The superdevice is declared into the component support, along with the
> subcomponents.  The superdevice receives callbacks to locate the
> subcomponents, and identify when all components are present.  At this
> point, we bind the superdevice, which causes the appropriate subsystem
> to be initialised in the conventional way.
>
> When any of the components or superdevice are removed from the system,
> we unbind the superdevice, thereby taking the subsystem down.
>
> Signed-off-by: Russell King 
> ---
>  drivers/base/Makefile |2 +-
>  drivers/base/component.c  |  379 
> +
>  include/linux/component.h |   31 
>  3 files changed, 411 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/base/component.c
>  create mode 100644 include/linux/component.h
>
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 94e8a80e87f8..870ecfd503af 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -1,6 +1,6 @@
>  # Makefile for the Linux device tree
>
> -obj-y  := core.o bus.o dd.o syscore.o \
> +obj-y  := component.o core.o bus.o dd.o syscore.o \
>driver.o class.o platform.o \
>cpu.o firmware.o init.o map.o devres.o \
>attribute_container.o transport_class.o \
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> new file mode 100644
> index ..5492cd8d2247
> --- /dev/null
> +++ b/drivers/base/component.c
> @@ -0,0 +1,379 @@
> +/*
> + * Componentized device handling.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This is work in progress.  We gather up the component devices into a list,
> + * and bind them when instructed.  At the moment, we're specific to the DRM
> + * subsystem, and only handles one master device, but this doesn't have to be
> + * the case.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct master {
> +   struct list_head node;
> +   struct list_head components;
> +   bool bound;
> +
> +   const struct component_master_ops *ops;
> +   struct device *dev;
> +};
> +
> +struct component {
> +   struct list_head node;
> +   struct list_head master_node;
> +   struct master *master;
> +   bool bound;
> +
> +   const struct component_ops *ops;
> +   struct device *dev;
> +};
> +
> +static DEFINE_MUTEX(component_mutex);
> +static LIST_HEAD(component_list);
> +static LIST_HEAD(masters);
> +
> +static struct master *__master_find(struct device *dev, const struct 
> component_master_ops *ops)
> +{
> +   struct master *m;
> +
> +   list_for_each_entry(m, &masters, node)
> +   if (m->dev == dev && (!ops || m->ops == ops))
> +   return m;
> +
> +   return NULL;
> +}
> +
> +/* Attach an unattached component to a master. */
> +static void component_attach_master(struct master *master, struct component 
> *c)
> +{
> +   c->master = master;
> +
> +   list_add_tail(&c->master_node, &master->components);
> +}
> +
> +/* Detach a component from a master. */
> +static void component_detach_master(struct master *master, struct component 
> *c)
> +{
&g

Re: [PATCH RFC 26/46] drivers/base: provide an infrastructure for componentised subsystems

[Sean Paul]

On Wed, Jan 8, 2014 at 4:36 PM, Russell King - ARM Linux
 wrote:
> On Tue, Jan 07, 2014 at 03:18:21PM -0500, Sean Paul wrote:
>> On Thu, Jan 2, 2014 at 4:27 PM, Russell King
>>  wrote:
>> > Subsystems such as ALSA, DRM and others require a single card-level
>> > device structure to represent a subsystem.  However, firmware tends to
>> > describe the individual devices and the connections between them.
>> >
>> > Therefore, we need a way to gather up the individual component devices
>> > together, and indicate when we have all the component devices.
>> >
>> > We do this in DT by providing a "superdevice" node which specifies
>> > the components, eg:
>> >
>> > imx-drm {
>> > compatible = "fsl,drm";
>> > crtcs = <&ipu1>;
>> > connectors = <&hdmi>;
>> > };
>> >
>>
>> Hi Russell,
>> This looks really good. I'd definitely like to use it for the exynos
>> drm driver both to bind the various IP blocks together, and also to
>> pull in any attached bridges that are specified in the device tree.
>> Along those lines, it might be worthwhile to pull some of the master
>> bind functionality in your next patch into drm helpers so other
>> drivers can use them, and so we have concrete bindings across drm.
>
> Which bits do you think would be useful to move into the core?
> imx_drm_add_components() is rather imx-drm specific, especially for
> the CRTCs - it carries the knowledge that the OF device to be matched
> can be found in the _parent_ device, rather than the device registered
> into the component helpers.
>

Yeah, I was thinking of imx_drm_add_components() actually. It probably
doesn't make sense to enforce the parent relationship in a generic
manner, but it would be nice to have a helper which added the various
drm components (crtc/encoder/bridge/connector) with a consistent
binding.

We have 3 different exynos boards which would have the following
superdevices (please forgive my hypothetical syntax/naming):

Board 1:
exynos-drm {
compatible = "exynos,drm";
crtcs = <&fimd1>, <&mixer1>;
encoders = <&dp1>, <&hdmi1>;
bridges = <&ptn3460 &dp1>;
connectors = <&ptn3460>, <&hdmi1>;
};

Board 2:
exynos-drm {
compatible = "exynos,drm";
crtcs = <&fimd1>, <&mixer1>;
encoders = <&dp1>, <&hdmi1>;
bridges = <&ps8622 &dp1>, <&anx7808 &hdmi1>;
connectors = <&ps8622>, <&anx7808>;
};

Board 3:
exynos-drm {
compatible = "exynos,drm";
crtcs = <&fimd1>, <&mixer1>;
encoders = <&dp1>, <&hdmi1>;
connectors = <&dp1>, <&hdmi1>;
};

In addition to enforcing common bindings across drivers, we could also
assign bridges to encoders from the dt. The bridge->encoder
relationship is 1:1 at the moment, and the bridge driver can be a
completely separate entity from the encoder. Allowing assignment from
the dt means the encoder never needs to know whether a bridge is
connected.

Right now the encoder must enumerate all possible bridges to see if
they are present (check out "[PATCH v3 31/32] drm/exynos: Move lvds
bridge discovery into DP driver" for an example of what I mean).

Sean

> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: r8192e_pci driver broken 3.14+

[Sean MacLennan]

On Sat, 19 Apr 2014 16:57:45 -0400
Sean MacLennan  wrote:

> A sparse error fixup removed a htons() which is required for the
> driver to function. This patch puts the htons() back and fixes the
> sparse warning correctly by changing the left side cast.
> 
> Signed-off-by: Sean MacLennan 
> ---
> diff --git a/drivers/staging/rtl8192e/rtllib_tx.c
> b/drivers/staging/rtl8192e/rtllib_tx.c index 11d0a9d..b7dd153 100644
> --- a/drivers/staging/rtl8192e/rtllib_tx.c
> +++ b/drivers/staging/rtl8192e/rtllib_tx.c
> @@ -171,7 +171,7 @@ inline int rtllib_put_snap(u8 *data, u16 h_proto)
>   snap->oui[1] = oui[1];
>   snap->oui[2] = oui[2];
>  
> - *(u16 *)(data + SNAP_SIZE) = h_proto;
> + *(__be16 *)(data + SNAP_SIZE) = htons(h_proto);
>  
>   return SNAP_SIZE + sizeof(u16);
>  }

Any status on this patch?

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

[PATCH] r8192e_pci driver broken 3.14+

[Sean MacLennan]

A sparse error fixup removed a htons() which is required for the driver
to function. This patch puts the htons() back and fixes the sparse
warning correctly by changing the left side cast.

Signed-off-by: Sean MacLennan 
---
diff --git a/drivers/staging/rtl8192e/rtllib_tx.c
b/drivers/staging/rtl8192e/rtllib_tx.c index 11d0a9d..b7dd153 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -171,7 +171,7 @@ inline int rtllib_put_snap(u8 *data, u16 h_proto)
snap->oui[1] = oui[1];
snap->oui[2] = oui[2];
 
-   *(u16 *)(data + SNAP_SIZE) = h_proto;
+   *(__be16 *)(data + SNAP_SIZE) = htons(h_proto);
 
return SNAP_SIZE + sizeof(u16);
 }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c

[Sean Wang]

On Mon, 2018-04-02 at 09:34 +1000, NeilBrown wrote:
> On Thu, Mar 29 2018, Chris Coffey wrote:
> 
> > This fixes the following sparse warning:
> >
> > drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
> > 'mtk_set_ethtool_ops' was not declared. Should it be static?
> >
> > Signed-off-by: Chris Coffey 
> 
> Reviewed-by: NeilBrown 
> 
> Thanks,
> NeilBrown
> 
Hi, Neil

Forgive me I cannot find the cover letter in the original series in my
mailbox to make a reply, so I rudely made here just letting you know
something good to the growth of mt7621 support in upstream.

do you have maintained an out-of-tree branch to boot the mt7621 machine
with those staging patches?

If so, it would become a bit easier for me that maybe I could give a
hand for migrating these staging driver for mt7621 to mainline. I
thought mmc, pci, ethernet, gsw and hsdma all could probably reuse the
current mainline code.

Sean
> 
> > ---
> > Changes in v2:
> >  - Per GregKH's feedback (thanks!), don't add unnecessary new .h file
> >  dependencies. This patch version reverts those changes and fixes the
> >  problem directly in ethtool.c (which is that it didn't include
> >  ethtool.h anywhere -- mtk_set_ethtool_ops is not static).
> >
> >  drivers/staging/mt7621-eth/ethtool.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/mt7621-eth/ethtool.c 
> > b/drivers/staging/mt7621-eth/ethtool.c
> > index 38ba0c040a..5268c5ca09 100644
> > --- a/drivers/staging/mt7621-eth/ethtool.c
> > +++ b/drivers/staging/mt7621-eth/ethtool.c
> > @@ -13,6 +13,7 @@
> >   */
> >  
> >  #include "mtk_eth_soc.h"
> > +#include "ethtool.h"
> >  
> >  static const char mtk_gdma_str[][ETH_GSTRING_LEN] = {
> >  #define _FE(x...)  # x,
> > -- 
> > 2.11.0
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek


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

[PATCH] Staging: Remove a few superfluous braces

[Sean Williams]

Please don't flame me :) I'm getting my feet wet with kernel contribution.
One example I saw in a video by GKH suggested cleaning up coding style as a 
good first commit.

Signed-off-by: Sean Williams 
---
 drivers/staging/comedi/drivers/ni_at_a2150.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_at_a2150.c 
b/drivers/staging/comedi/drivers/ni_at_a2150.c
index 2512ce8..c77d343 100644
--- a/drivers/staging/comedi/drivers/ni_at_a2150.c
+++ b/drivers/staging/comedi/drivers/ni_at_a2150.c
@@ -684,13 +684,12 @@ static int a2150_set_chanlist(struct comedi_device *dev,
devpriv->config_bits |= CHANNEL_BITS(0x4 | start_channel);
break;
case 2:
-   if (start_channel == 0) {
+   if (start_channel == 0)
devpriv->config_bits |= CHANNEL_BITS(0x2);
-   } else if (start_channel == 2) {
+   else if (start_channel == 2)
devpriv->config_bits |= CHANNEL_BITS(0x3);
-   } else {
+   else
return -1;
-   }
break;
case 4:
devpriv->config_bits |= CHANNEL_BITS(0x1);
-- 
1.8.1.2

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

Re: [V2 PATCH 03/10] added media agnostic (MA) data structures and handling

[Sean O. Stalley]

On Tue, Nov 11, 2014 at 01:38:21PM +0900, Greg KH wrote:
> On Mon, Nov 10, 2014 at 06:09:34PM -0800, Stephanie Wallick wrote:
> Intel has a whole group of very experienced Linux kernel developers who
> will review code before you sent it out publicly.  Please take advantage
> of them and run this all through them before resending this out again.
> 
> If you did run this code through that group, please let me know who it
> was specifically that allowed this stuff to get through, and why they
> didn't want their name on this code submission.  I need to have a strong
> word with them...

We submitted the patches for internal review and got no objections to
release. We will be more aggressive in seeking out feedback (and approval)
before resubmitting any code.
 
> Yes, I am holding you to a higher standard than staging code normally
> is, and yes, it is purely because of the company you work for.  But I
> only do that because your company knows how to do this stuff right, and
> you have access to the resources and talent to help make this code
> right.  Other people and companies do not have the kind of advantage
> that you do.

We know we are fortunate to work for a company with so much talent and
resources and we don't mind being held to a higher standard. We have been
receiving multiple requests for our host driver and wanted to make it
available as soon as possible for others to use. We thought putting our
host driver into staging would be a good way to release it, but realize now
that it was premature. 

> Wasting community member's time (i.e. mine) by forcing _them_ to review
> stuff like this, is something that your company knows better than to do,
> as should you as well.
> 
> I want to see some more senior Intel kernel developer's signed-off-by
> lines on this code before I will ever consider accepting it for the
> kernel.  Please do not resend this code until that happens.
> 
> greg k-h

We apologize for wasting everyone's time and will certainly learn from this.
We won't resubmit the driver until a senior kernel developer has signed off on 
it.

Sincerely,
Sean O. Stalley
Stephanie S. Wallick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [V2 PATCH 02/10] added media agnostic (MA) USB HCD roothubs

[Sean O. Stalley]

Thank you for your review. My responses are inline.

Greg has requested that we clean up the driver internally before
we resubmit another patchset to the mailing list. I will make
sure the changes you requested make it in, but it may be a while
before you see a patchset with the fixes included.

Thanks,
Sean O. Stalley

On Wed, Nov 12, 2014 at 09:35:42AM +0100, Oliver Neukum wrote:
> On Mon, 2014-11-10 at 18:09 -0800, Stephanie Wallick wrote:
> > diff --git a/drivers/staging/mausb/drivers/mausb_hub.c 
> > b/drivers/staging/mausb/drivers/mausb_hub.c
> > new file mode 100644
> > index 000..63c0fe4
> > --- /dev/null
> > +++ b/drivers/staging/mausb/drivers/mausb_hub.c
> 
> > +/**
> > + * Returns true if the given is the superspeed HCD. Note: The primary HCD 
> > is
> > + * High Speed and the shared HCD is SuperSpeed.
> > + */
> 
> Why in that order?
> 

We should probably switch this & make the superspeed hub primary.
That way we match the xhci driver.

> > +bool mausb_is_ss_hcd(struct usb_hcd *hcd)
> > +{
> > +   if (usb_hcd_is_primary_hcd(hcd))
> > +   return false;
> > +   else
> > +   return true;
> > +}
> 
> 
> 
> > +
> > +/**
> > + * Called by usb core when polling for a port status change.
> > + *
> > + * @hcd:   USB HCD being polled.
> > + * @buf:   Holds port status changes (if any).
> > + *
> > + * Returns zero if there is no status change, otherwise returns number of
> > + * bytes in buf. When there is a status change on a port, the bit indexed
> > + * at the port number + 1 (e.g. bit 2 for port 1) is set in the buffer.
> > + */
> > +int mausb_hub_status_data(struct usb_hcd *hcd, char *buf)
> > +{
> > +   int  i;
> > +   u16  port_change = 0;
> > +   u32  status = 0;
> > +   int  ret = 1;
> > +   struct mausb_hcd *mhcd = usb_hcd_to_mausb_hcd(hcd);
> > +   struct mausb_root_hub*roothub = usb_hcd_to_roothub(hcd);
> > +
> > +   /*
> > +* Buf should never be more that 2 bytes. USB 3.0 hubs cannot have
> > +* more than 15 downstream ports.
> > +*/
> > +   buf[0] = 0;
> > +   if (MAUSB_ROOTHUB_NUM_PORTS > 7) {
> > +   buf[1] = 0;
> > +   ret++;
> > +   }
> 
> Endianness bug.
> 

Could you elaborate?
It was my understanding that this buffer was host-endian.
Is this an unacceptable way to clear the buffer?

> > +
> > +   for (i = 0; i < MAUSB_ROOTHUB_NUM_PORTS; i++) {
> > +   port_change = roothub->port_status[i].wPortChange;
> > +   if (port_change)
> > +   status |= (1 << (i + 1));
> > +   }
> > +
> > +   mausb_dbg(mhcd, "%s: hub status is 0x%x\n", __func__, status);
> > +
> > +   /* hcd might be suspended, resume if there is a status change */
> > +   if (mhcd->disabled == 0) {
> > +   if ((hcd->state == HC_STATE_SUSPENDED) && status)
> > +   usb_hcd_resume_root_hub(hcd);
> > +   }
> > +
> > +   memcpy(buf, (char *)&status, ret);
> > +
> > +   return status ? ret : 0;
> > +}
> > +
> > +/**
> > + * Sets the bitfields in the hub descriptor of the 2.0 root hub. Always
> > + * returns zero.
> > + */
> > +int mausb_set_hub_descriptor(struct usb_hub_descriptor *hub_des)
> > +{
> > +   /* set the values to the default */
> > +   hub_des->bDescLength  = sizeof(struct usb_hub_descriptor);
> > +   hub_des->bDescriptorType  = USB_DT_HUB;
> > +   hub_des->bNbrPorts= MAUSB_ROOTHUB_NUM_PORTS;
> > +   hub_des->wHubCharacteristics  = MAUSB_ROOTHUB_CHAR;
> > +   hub_des->bPwrOn2PwrGood   = MAUSB_ROOTHUB_PWR_ON_2_PWR_GOOD;
> > +   hub_des->bHubContrCurrent = MAUSB_ROOTHUB_CONTR_CURRENT;
> 
> Is that descriptor in bus or host endianness?
> 

All of the fields are little-endian. We should be using cpu_to_le16()
when setting wHubCharacteristics.

> > +
> > +   return 0;
> > +}
> > +
> > +/**
> > + * Sets the bitfields in the hub descriptor of the 3.0 root hub. Always
> > + * returns zero.
> 
> Then why return anything?
> 

Good point. I will change this (and mausb_set_hub_descriptor()) to be void.

> > + */
> > +int mausb_set_ss_hub_descriptor(struct usb_hub_descriptor *hub_des)
> > +{
> > +   /* set the values to the default */
> > +   hub_des->bDescLength  = sizeof(struct usb_hub_descriptor);
> > +   hub_des->bDescriptorTy

Re: [PATCH 05/10] added media specific (MS) TCP drivers

[Sean O. Stalley]

Thank You for reviewing our code.

I believe most of the problems you pointed out in mausb_ioctl.c
were addressed in [V2 PATCH 5/10]. I am working on adding the proper
error checking to the TCP drivers.

Greg has requested that we clean up our code internally before
submitting another patchset to the mailing list. I will make sure
we fix the problems you pointed out, but it may be a while before
you see another patchset.

Thanks,
Sean

On Tue, Nov 04, 2014 at 09:48:33AM +0100, Tobias Klauser wrote:
> On 2014-11-03 at 21:42:52 +0100, Stephanie Wallick 
>  wrote:
> > This is where we handle media specific packets and transport. The MS driver
> > interfaces with a media agnostic (MA) driver via a series of transfer pairs.
> > Transfer pairs consist of a set of functions to pass MA USB packets back
> > and forth between MA and MS drivers. There is one transfer pair per device
> > endpoint and one transfer pair for control/management traffic. When the MA
> > driver needs to send an MA USB packet, it hands the packet off to the MS
> > layer where the packet is converted into an MS form and sent via TCP over
> > the underlying ethernet or wireless medium. When the MS driver receives a
> > packet, it converts it into an MA USB packet and hands it off the the MA
> > driver for handling.
> > 
> > In addition, the MS driver provides an interface to inititate connection 
> > events.
> > Because there are no physical MA USB ports in an MA USB host, the host must 
> > be
> > notified via software when a device is connected.
> > 
> > Lastly, the MS driver contains a number of ioctl functions that are used by 
> > a
> > utility to adjust medium-related driver parameters and connect or 
> > disconnect the
> > MA USB host and device drivers.
> > 
> > Signed-off-by: Sean O. Stalley 
> > Signed-off-by: Stephanie Wallick 
> > ---
> >  drivers/staging/mausb/drivers/mausb_ioctl.c  | 373 +++
> >  drivers/staging/mausb/drivers/mausb_ioctl.h  |  99 +
> >  drivers/staging/mausb/drivers/mausb_msapi.c  | 110 ++
> >  drivers/staging/mausb/drivers/mausb_msapi.h  | 232 
> >  drivers/staging/mausb/drivers/mausb_tcp-device.c | 147 
> >  drivers/staging/mausb/drivers/mausb_tcp-host.c   | 144 
> >  drivers/staging/mausb/drivers/mausb_tcp.c| 446 
> > +++
> >  drivers/staging/mausb/drivers/mausb_tcp.h| 129 +++
> >  8 files changed, 1680 insertions(+)
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_ioctl.c
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_ioctl.h
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_msapi.c
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_msapi.h
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_tcp-device.c
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_tcp-host.c
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_tcp.c
> >  create mode 100644 drivers/staging/mausb/drivers/mausb_tcp.h
> > 
> > diff --git a/drivers/staging/mausb/drivers/mausb_ioctl.c 
> > b/drivers/staging/mausb/drivers/mausb_ioctl.c
> > new file mode 100644
> > index 000..0c6c6bd
> > --- /dev/null
> > +++ b/drivers/staging/mausb/drivers/mausb_ioctl.c
> 
> [...]
> 
> > +/**
> > + * This function is used to send a message to the user, in other words, the
> > + * calling process. It basically copies the message one byte at a time.
> > + *
> > + * @msg:   The message to be sent to the user.
> > + * @buffer:The buffer in which to put the message. This buffer was 
> > given to
> > + * us to fill.
> > + */
> > +void to_user(char *msg, long unsigned int buffer)
> > +{
> > +   int length = (int)strlen(msg);
> > +   int bytes = 0;
> > +
> > +   while (length && *msg) {
> > +   put_user(*(msg++), (char *)buffer++);
> > +   length--;
> > +   bytes++;
> > +   }
> 
> Any reason not to use copy_to_user here? That way, access_ok would only
> need to be executed once for the whole range.
> 
> In any case, the return value of put_user/copy_to_user will need to be
> checked.
> 
> > +
> > +   put_user('\0', (char *)buffer + bytes);
> > +}
> 
> [...]
> 
> > +/**
> > + * This function is used to read from the device file. From the 
> > perspective of
> > + * the device, the user is reading information from us. This is one of the
> > + * entry points to this module.
> > + *
> > + * @file:  The device file. W

Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver

[Sean O. Stalley]

Thanks for reviewing. My responses are inline.

Greg has asked that we clean up this code internally before we
send out another patchset to the mailing list. I will address
the issues you pointed out, but it may be a while before you see
another patchset.

Thanks Again,
Sean

On Tue, Nov 11, 2014 at 10:54:30AM -0500, Alan Stern wrote:
> On Mon, 10 Nov 2014, Stephanie Wallick wrote:
> 
> > +static struct mausb_hcd mhcd;
> 
> Only one statically-allocated structure?  What if somebody wants to 
> have more than one of these things in their system?
> 

Our plan to support multiple MA devices is to have them all connected
to the same virtual host controller, so only 1 would be needed.

Would you prefer we have 1 host controller instance per MA device?
We are definitely open to suggestions on how this should be architected.

> > +/**
> > + * @maurb: Media agnostic structure with URB to release.
> > + * @status:Status for URB that is getting released.
> > + *
> > + * Removes an URB from the queue, deletes the media agnostic information in
> > + * the urb, and gives the URB back to the HCD. Caller must be holding the
> > + * driver's spinlock.
> > + */
> > +void mausb_unlink_giveback_urb(struct mausb_urb *maurb, int status)
> > +{
> > +   struct urb  *urb;
> > +   struct usb_hcd  *hcd;
> > +   struct api_context  *ctx = NULL;
> > +   unsigned long   irq_flags;
> > +
> > +   hcd = mausb_hcd_to_usb_hcd(&mhcd);
> > +
> > +   spin_lock_irqsave(&mhcd.giveback_lock, irq_flags);
> 
> Why do you need multiple spinlocks?  Isn't one lock sufficient?
> 
We will simplify the locking scheme before resubmitting.

I think it might be worthwhile to have a per-endpoint lock, see below.

> > +   if (!maurb) {
> > +   mausb_err(&mhcd, "%s: no maurb\n", __func__);
> > +   spin_unlock_irqrestore(&mhcd.giveback_lock, irq_flags);
> > +   return;
> > +   } else {
> > +   urb = maurb->urb;
> > +   ctx = urb->context;
> > +   }
> > +
> > +   if (!urb) {
> > +   mausb_err(&mhcd, "%s: no urb\n", __func__);
> > +   mausb_internal_drop_maurb(maurb, &mhcd);
> > +   spin_unlock_irqrestore(&mhcd.giveback_lock, irq_flags);
> > +   return;
> > +   }
> > +
> > +   mausb_dbg(&mhcd, "%s: returning urb with status %i\n", __func__, 
> > status);
> > +
> > +   usb_hcd_unlink_urb_from_ep(hcd, urb);
> > +   usb_hcd_giveback_urb(hcd, urb, status);
> 
> You must not call this function while holding any spinlocks.  What happens
> if the URB's completion routine tries to resubmit?
> 

This works with our multi-lock scheme, but I will fix when we move to 1 lock.

> > +
> > +   /* remove the mausb-specific data */
> > +   mausb_internal_drop_maurb(maurb, &mhcd);
> > +
> > +   spin_unlock_irqrestore(&mhcd.giveback_lock, irq_flags);
> > +}
> > +
> > +/**
> > + * Adds an URB to the endpoint queue then calls the URB handler. URB is 
> > wrapped
> > + * in media agnostic structure before being enqueued.
> > + */
> > +static int mausb_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
> > +   gfp_t memflags)
> > +{
> > +   int ret = 0;
> > +   struct mausb_urb*maurb;
> > +   struct mausb_host_ep*ep;
> > +   unsigned long   irq_flags;
> > +
> > +   if (!hcd || !urb) {
> > +   pr_err("%s: no %s\n", __func__, (hcd ? "urb" : "USB hcd"));
> > +   }
> 
> This can never happen.  The USB core guarantees it; you don't need 
> to check.
> 

I will remove this check (along with any other unnecessary checks for things
guaranteed from usbcore).

> > +   ep   = usb_to_ma_endpoint(urb->ep);
> > +
> > +   if (!ep) {
> > +   mausb_err(&mhcd, "%s: no endpoint\n", __func__);
> > +   return -EINVAL;
> > +   }
> > +
> > +   if (urb->status != -EINPROGRESS) {
> > +   mausb_err(&mhcd, "%s: urb already unlinked, status is %i\n",
> > +   __func__, urb->status);
> > +   return urb->status;
> > +   }
> 
> You also don't need to check this.
> 
Will remove.

> > +   /* If the endpoint isn't activated, we can't enqueue anything. */
> > +   if (MAUSB_EP_HANDLE_UNASSIGNED == ep->ep_handle_state) {
> > +   mausb_err(&mhcd, "%s: endpoint handle unassigned\n", __f

Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver

[Sean O. Stalley]

Sorry, for got to respond to a couple comments. See responses below.

On Wed, Nov 12, 2014 at 01:40:21PM -0800, Sean O. Stalley wrote:
> Thanks for reviewing. My responses are inline.
> 
> Greg has asked that we clean up this code internally before we
> send out another patchset to the mailing list. I will address
> the issues you pointed out, but it may be a while before you see
> another patchset.
> 
> Thanks Again,
> Sean
> 
> On Tue, Nov 11, 2014 at 10:54:30AM -0500, Alan Stern wrote:
> > On Mon, 10 Nov 2014, Stephanie Wallick wrote:
> > > + /* If the endpoint isn't activated, we can't enqueue anything. */
> > > + if (MAUSB_EP_HANDLE_UNASSIGNED == ep->ep_handle_state) {
> > > + mausb_err(&mhcd, "%s: endpoint handle unassigned\n", __func__);
> > > + return -EPIPE;
> > > + }
> > > +
> > > + if (USB_SPEED_FULL != urb->dev->speed) /* suppress checks */
> > > + ep->max_pkt = usb_endpoint_maxp(&urb->ep->desc);
> > 
> > What happens to full-speed devices?  Don't they have maxpacket values?
> > 

This was part of a work-around.
Per the MA spec (section 7.3.2.2), wMaxPacketSize should be initially
set to 8 for EP0 of FS devices. The usbcore sets it to 64.

This makes sure the EPHandleReq Packet has the per-spec values.

> > > + if (ret < 0) {
> > > + mausb_err(&mhcd, "urb enqueue failed: error %d\n", ret);
> > > + usb_hcd_unlink_urb_from_ep(hcd, urb);
> > > + return ret;
> > > + }
> > > +
> > > + /* get usb device and increment reference counter */
> > > + if (!mhcd.udev) {
> > > + mhcd.udev = urb->dev;
> > > + usb_get_dev(mhcd.udev);
> > > + }
> > 
> > What happens if more than one device is in use at a time?
> > 

This is wrong. This call should be in mausb_internal_alloc_dev(). Will fix.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver

[Sean O. Stalley]

On Wed, Nov 12, 2014 at 05:03:18PM -0500, Alan Stern wrote:
> On Wed, 12 Nov 2014, Sean O. Stalley wrote:
> > Our plan to support multiple MA devices is to have them all connected
> > to the same virtual host controller, so only 1 would be needed.
> > 
> > Would you prefer we have 1 host controller instance per MA device?
> > We are definitely open to suggestions on how this should be architected.
> 
> I haven't read the MA USB spec, so I don't know how it's intended to 
> work.  Still, what happens if you create a virtual host controller 
> with, say, 16 ports, and then someone wants to connect a 17th MA 
> device?

To summarize the spec:
MA USB groups a host & connected devices into MA service sets (MSS).
The architectural limit is 254 MA devices per MSS.

If the host needs to connect more devices than that, It can start a
new MSS and connect to 254 more MA devices.



Is supporting up to 254 devices on one machine sufficient?

Would it make sense (and does the usb stack support) having 254 root
ports on one host controller? If so, we could make our host
controller instance have 254 ports. I'm guessing the hub driver may have
a problem with this (especially for superspeed).

If that doesn't make sense (or isn't supported), we can have 1 host
controller instance per MA device. Would that be preferred?

> Also, I noticed that your patch adds a new bus type for these MA host 
> controllers.  It really seems like overkill to have a whole new bus 
> type if there's only going to be one device on it.

The bus was added when we were quickly trying to replace the platform
device code. It's probably not the right thing to do.

I'm still not sure why we can't make our hcd a platform device,
especially since dummy_hcd & the usbip's hcd are both platform devices.

> > If we get rid of these locks, endpoints can't run simultaneously.
> > MA USB IN endpoints have to copy data, which could take a while.
> 
> I don't know what you mean by "run simultaneously".  Certainly multiple 
> network packets can be transmitted and received concurrently even if 
> you use a single spinlock, since your locking won't affect the 
> networking subsystem.

I meant we couldn't have 2 threads in our driver. With one lock,
One thread would always have to wait for the other, even though
they could be working on 2 different endpoints doing completely
independent tasks.

> > Couldn't this cause a bottleneck?
> 
> Probably not enough to matter.  After all, the other host controller
> drivers rely on a single spinlock.  And if it did matter, you could
> drop the spinlock while copying the data.

Good point. We can cut our driver down to using 1 lock. If we find that
only having 1 spinlock does cause a bottleneck, we can deal with it then.


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