[PATCH 4/4] Staging: rts5208: sd: Fixed multiple coding style issues Fixed multiple braces issues

2018-09-28 Thread Maxime Desroches
---
 drivers/staging/rts5208/sd.c | 704 +--
 1 file changed, 268 insertions(+), 436 deletions(-)

diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
index e7efa34195c7..930c61ccb047 100644
--- a/drivers/staging/rts5208/sd.c
+++ b/drivers/staging/rts5208/sd.c
@@ -109,9 +109,8 @@ static int sd_check_data0_status(struct rtsx_chip *chip)
u8 stat;
 
retval = rtsx_read_register(chip, REG_SD_STAT1, );
-   if (retval) {
+   if (retval)
return retval;
-   }
 
if (!(stat & SD_DAT0_STATUS)) {
sd_set_err_code(chip, SD_BUSY);
@@ -234,9 +233,8 @@ static int sd_send_cmd_get_rsp(struct rtsx_chip *chip, u8 
cmd_idx,
if ((cmd_idx != SEND_RELATIVE_ADDR) &&
(cmd_idx != SEND_IF_COND)) {
if (cmd_idx != STOP_TRANSMISSION) {
-   if (ptr[1] & 0x80) {
+   if (ptr[1] & 0x80)
return STATUS_FAIL;
-   }
}
 #ifdef SUPPORT_SD_LOCK
if (ptr[1] & 0x7D) {
@@ -284,9 +282,8 @@ static int sd_read_data(struct rtsx_chip *chip,
if (!buf)
buf_len = 0;
 
-   if (buf_len > 512) {
+   if (buf_len > 512)
return STATUS_FAIL;
-   }
 
rtsx_init_cmd(chip);
 
@@ -331,9 +328,8 @@ static int sd_read_data(struct rtsx_chip *chip,
 
if (buf && buf_len) {
retval = rtsx_read_ppbuf(chip, buf, buf_len);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
}
 
return STATUS_SUCCESS;
@@ -359,9 +355,8 @@ static int sd_write_data(struct rtsx_chip *chip, u8 
trans_mode,
 
if (buf && buf_len) {
retval = rtsx_write_ppbuf(chip, buf, buf_len);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
}
 
rtsx_init_cmd(chip);
@@ -426,9 +421,8 @@ static int sd_check_csd(struct rtsx_chip *chip, char 
check_wp)
break;
}
 
-   if (i == 6) {
+   if (i == 6)
return STATUS_FAIL;
-   }
 
memcpy(sd_card->raw_csd, rsp + 1, 15);
 
@@ -543,9 +537,8 @@ static int sd_set_sample_push_timing(struct rtsx_chip *chip)
}
 
retval = rtsx_write_register(chip, REG_SD_CFG1, 0x1C, val);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -606,9 +599,8 @@ static int sd_set_clock_divider(struct rtsx_chip *chip, u8 
clk_div)
val = 0x20;
 
retval = rtsx_write_register(chip, REG_SD_CFG1, mask, val);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -619,16 +611,14 @@ static int sd_set_init_para(struct rtsx_chip *chip)
int retval;
 
retval = sd_set_sample_push_timing(chip);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
sd_choose_proper_clock(chip);
 
retval = switch_clock(chip, sd_card->sd_clock);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
return STATUS_SUCCESS;
 }
@@ -651,9 +641,8 @@ int sd_select_card(struct rtsx_chip *chip, int select)
}
 
retval = sd_send_cmd_get_rsp(chip, cmd_idx, addr, cmd_type, NULL, 0);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
return STATUS_SUCCESS;
 }
@@ -667,9 +656,8 @@ static int sd_update_lock_status(struct rtsx_chip *chip)
 
retval = sd_send_cmd_get_rsp(chip, SEND_STATUS, sd_card->sd_addr,
 SD_RSP_TYPE_R1, rsp, 5);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
if (rsp[1] & 0x02)
sd_card->sd_lock_status |= SD_LOCKED;
@@ -679,9 +667,8 @@ static int sd_update_lock_status(struct rtsx_chip *chip)
dev_dbg(rtsx_dev(chip), "sd_card->sd_lock_status = 0x%x\n",
sd_card->sd_lock_status);
 
-   if (rsp[1] & 0x01) {
+   if (rsp[1] & 0x01)
return STATUS_FAIL;
-   }
 
return STATUS_SUCCESS;
 }
@@ -698,9 +685,8 @@ static int sd_wait_state_data_ready(struct rtsx_chip *chip, 
u8 state,
retval = sd_send_cmd_get_rsp(chip, SEND_STATUS,
 sd_card->sd_addr, SD_RSP_TYPE_R1,
 rsp, 5);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != 

[PATCH 3/3] Staging: rts5208: rtsx_scsi: Fixed multiple coding style issues Fized multiple braces issues

2018-09-28 Thread Maxime Desroches
---
 drivers/staging/rts5208/rtsx_scsi.c | 108 ++--
 1 file changed, 36 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx_scsi.c 
b/drivers/staging/rts5208/rtsx_scsi.c
index c9a6d97938f6..9c594a778425 100644
--- a/drivers/staging/rts5208/rtsx_scsi.c
+++ b/drivers/staging/rts5208/rtsx_scsi.c
@@ -507,9 +507,8 @@ static int inquiry(struct scsi_cmnd *srb, struct rtsx_chip 
*chip)
}
 
buf = vmalloc(scsi_bufflen(srb));
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
 #ifdef SUPPORT_MAGIC_GATE
if ((chip->mspro_formatter_enable) &&
@@ -637,9 +636,8 @@ static int request_sense(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
}
 
buf = vmalloc(scsi_bufflen(srb));
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
tmp = (unsigned char *)sense;
memcpy(buf, tmp, scsi_bufflen(srb));
@@ -783,9 +781,8 @@ static int mode_sense(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
 #endif
 
buf = kmalloc(data_size, GFP_KERNEL);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
page_code = srb->cmnd[2] & 0x3f;
 
@@ -999,9 +996,8 @@ static int read_format_capacity(struct scsi_cmnd *srb, 
struct rtsx_chip *chip)
buf_len = (scsi_bufflen(srb) > 12) ? 0x14 : 12;
 
buf = kmalloc(buf_len, GFP_KERNEL);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
buf[i++] = 0;
buf[i++] = 0;
@@ -1076,9 +1072,8 @@ static int read_capacity(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
}
 
buf = kmalloc(8, GFP_KERNEL);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
card_size = get_card_size(chip, lun);
buf[0] = (unsigned char)((card_size - 1) >> 24);
@@ -1116,9 +,8 @@ static int read_eeprom(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
len = ((u16)srb->cmnd[4] << 8) | srb->cmnd[5];
 
buf = vmalloc(len);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
retval = rtsx_force_power_on(chip, SSC_PDCTL);
if (retval != STATUS_SUCCESS) {
@@ -1180,9 +1174,8 @@ static int write_eeprom(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
len = (unsigned short)min_t(unsigned int, scsi_bufflen(srb),
len);
buf = vmalloc(len);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
rtsx_stor_get_xfer_buf(buf, len, srb);
scsi_set_resid(srb, scsi_bufflen(srb) - len);
@@ -1227,9 +1220,8 @@ static int read_mem(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
}
 
buf = vmalloc(len);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
retval = rtsx_force_power_on(chip, SSC_PDCTL);
if (retval != STATUS_SUCCESS) {
@@ -1282,9 +1274,8 @@ static int write_mem(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
 
len = (unsigned short)min_t(unsigned int, scsi_bufflen(srb), len);
buf = vmalloc(len);
-   if (!buf) {
+   if (!buf)
return TRANSPORT_ERROR;
-   }
 
rtsx_stor_get_xfer_buf(buf, len, srb);
scsi_set_resid(srb, scsi_bufflen(srb) - len);
@@ -1702,41 +1693,35 @@ static int set_chip_mode(struct scsi_cmnd *srb, struct 
rtsx_chip *chip)
if (phy_debug_mode) {
chip->phy_debug_mode = 1;
retval = rtsx_write_register(chip, CDRESUMECTL, 0x77, 0);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return TRANSPORT_FAILED;
-   }
 
rtsx_disable_bus_int(chip);
 
retval = rtsx_read_phy_register(chip, 0x1C, );
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return TRANSPORT_FAILED;
-   }
 
reg |= 0x0001;
retval = rtsx_write_phy_register(chip, 0x1C, reg);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return TRANSPORT_FAILED;
-   }
} else {
chip->phy_debug_mode = 0;
retval = rtsx_write_register(chip, CDRESUMECTL, 0x77, 0x77);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return TRANSPORT_FAILED;
-   }
 
rtsx_enable_bus_int(chip);
 
retval = rtsx_read_phy_register(chip, 0x1C, );
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return TRANSPORT_FAILED;
-   }
 

[PATCH 2/2] Staging: rts5208: rtsx_chip: Fixed multiple coding style issues

2018-09-28 Thread Maxime Desroches
Fixed multiple braces issues
Signed-off-by: Maxime Desroches 
---
 drivers/staging/rts5208/rtsx_chip.c | 463 
 1 file changed, 199 insertions(+), 264 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx_chip.c 
b/drivers/staging/rts5208/rtsx_chip.c
index 6b1234bff09c..6bec2ddc75f2 100644
--- a/drivers/staging/rts5208/rtsx_chip.c
+++ b/drivers/staging/rts5208/rtsx_chip.c
@@ -116,34 +116,31 @@ static int rtsx_pre_handle_sdio_old(struct rtsx_chip 
*chip)
 0xFF,
 MS_INS_PU | SD_WP_PU |
 SD_CD_PU | SD_CMD_PU);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
} else {
retval = rtsx_write_register(chip, FPGA_PULL_CTL,
 0xFF,
 FPGA_SD_PULL_CTL_EN);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
}
retval = rtsx_write_register(chip, CARD_SHARE_MODE, 0xFF,
 CARD_SHARE_48_SD);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
/* Enable SDIO internal clock */
retval = rtsx_write_register(chip, 0xFF2C, 0x01, 0x01);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
retval = rtsx_write_register(chip, SDIO_CTRL, 0xFF,
 SDIO_BUS_CTRL | SDIO_CD_CTRL);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
chip->sd_int = 1;
chip->sd_io = 1;
@@ -164,16 +161,16 @@ static int rtsx_pre_handle_sdio_new(struct rtsx_chip 
*chip)
if (chip->driver_first_load) {
if (CHECK_PID(chip, 0x5288)) {
retval = rtsx_read_register(chip, 0xFE5A, );
-   if (retval) {
+   if (retval)
return retval;
-   }
+
if (tmp & 0x08)
sw_bypass_sd = true;
} else if (CHECK_PID(chip, 0x5208)) {
retval = rtsx_read_register(chip, 0xFE70, );
-   if (retval) {
+   if (retval)
return retval;
-   }
+
if (tmp & 0x80)
sw_bypass_sd = true;
}
@@ -192,9 +189,9 @@ static int rtsx_pre_handle_sdio_new(struct rtsx_chip *chip)
u8 cd_toggle_mask = 0;
 
retval = rtsx_read_register(chip, TLPTISTAT, );
-   if (retval) {
+   if (retval)
return retval;
-   }
+
cd_toggle_mask = 0x08;
 
if (tmp & cd_toggle_mask) {
@@ -202,22 +199,20 @@ static int rtsx_pre_handle_sdio_new(struct rtsx_chip 
*chip)
if (CHECK_PID(chip, 0x5288)) {
retval = rtsx_write_register(chip, 0xFE5A,
 0x08, 0x00);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
} else if (CHECK_PID(chip, 0x5208)) {
retval = rtsx_write_register(chip, 0xFE70,
 0x80, 0x00);
-   if (retval) {
+   if (retval)
return retval;
-   }
}
 
retval = rtsx_write_register(chip, TLPTISTAT, 0xFF,
 tmp);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
chip->need_reset |= SD_CARD;
} else {
@@ -225,36 +220,35 @@ static int rtsx_pre_handle_sdio_new(struct rtsx_chip 
*chip)
 
if (chip->asic_code) {
retval = sd_pull_ctl_enable(chip);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
+

[PATCH 1/2] Staging: rts5208: rtsx_card: Fixed multiple coding style issues

2018-09-28 Thread Maxime Desroches
Fixed multiple coding style issues

Signed-off-by: Maxime Desroches 
---
 drivers/staging/rts5208/rtsx_card.c | 96 +++--
 1 file changed, 37 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx_card.c 
b/drivers/staging/rts5208/rtsx_card.c
index d26a8e372fce..b45abbe29bc2 100644
--- a/drivers/staging/rts5208/rtsx_card.c
+++ b/drivers/staging/rts5208/rtsx_card.c
@@ -647,9 +647,8 @@ int switch_ssc_clock(struct rtsx_chip *chip, int clk)
dev_dbg(rtsx_dev(chip), "Switch SSC clock to %dMHz (cur_clk = %d)\n",
clk, chip->cur_clk);
 
-   if ((clk <= 2) || (n > max_n)) {
+   if ((clk <= 2) || (n > max_n))
return STATUS_FAIL;
-   }
 
mcu_cnt = (u8)(125 / clk + 3);
if (mcu_cnt > 7)
@@ -688,15 +687,13 @@ int switch_ssc_clock(struct rtsx_chip *chip, int clk)
}
 
retval = rtsx_send_cmd(chip, 0, WAIT_TIME);
-   if (retval < 0) {
+   if (retval < 0)
return STATUS_ERROR;
-   }
 
udelay(10);
retval = rtsx_write_register(chip, CLK_CTL, CLK_LOW_FREQ, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
chip->cur_clk = clk;
 
@@ -790,49 +787,44 @@ int switch_normal_clock(struct rtsx_chip *chip, int clk)
}
 
retval = rtsx_write_register(chip, CLK_CTL, 0xFF, CLK_LOW_FREQ);
-   if (retval) {
+   if (retval)
return retval;
-   }
if (sd_vpclk_phase_reset) {
retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
 PHASE_NOT_RESET, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
retval = rtsx_write_register(chip, SD_VPCLK1_CTL,
 PHASE_NOT_RESET, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
}
retval = rtsx_write_register(chip, CLK_DIV, 0xFF,
 (div << 4) | mcu_cnt);
-   if (retval) {
+   if (retval)
return retval;
-   }
retval = rtsx_write_register(chip, CLK_SEL, 0xFF, sel);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
if (sd_vpclk_phase_reset) {
udelay(200);
retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
 PHASE_NOT_RESET, PHASE_NOT_RESET);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
retval = rtsx_write_register(chip, SD_VPCLK1_CTL,
 PHASE_NOT_RESET, PHASE_NOT_RESET);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
udelay(200);
}
retval = rtsx_write_register(chip, CLK_CTL, 0xFF, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
chip->cur_clk = clk;
 
@@ -878,9 +870,8 @@ int enable_card_clock(struct rtsx_chip *chip, u8 card)
clk_en |= MS_CLK_EN;
 
retval = rtsx_write_register(chip, CARD_CLK_EN, clk_en, clk_en);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -898,9 +889,8 @@ int disable_card_clock(struct rtsx_chip *chip, u8 card)
clk_en |= MS_CLK_EN;
 
retval = rtsx_write_register(chip, CARD_CLK_EN, clk_en, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -924,9 +914,8 @@ int card_power_on(struct rtsx_chip *chip, u8 card)
rtsx_add_cmd(chip, WRITE_REG_CMD, CARD_PWR_CTL, mask, val1);
 
retval = rtsx_send_cmd(chip, 0, 100);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
udelay(chip->pmos_pwr_on_interval);
 
@@ -934,9 +923,8 @@ int card_power_on(struct rtsx_chip *chip, u8 card)
rtsx_add_cmd(chip, WRITE_REG_CMD, CARD_PWR_CTL, mask, val2);
 
retval = rtsx_send_cmd(chip, 0, 100);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
return STATUS_SUCCESS;
 }
@@ -955,9 +943,8 @@ int card_power_off(struct rtsx_chip *chip, u8 card)
}
 
retval = rtsx_write_register(chip, CARD_PWR_CTL, mask, val);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -969,9 +956,8 @@ int card_rw(struct scsi_cmnd *srb, struct rtsx_chip *chip,
unsigned int lun = SCSI_LUN(srb);
int i;
 
-   if (!chip->rw_card[lun]) {
+   if (!chip->rw_card[lun])

RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-28 Thread Y.b. Lu
Hi Andrew,

> -Original Message-
> From: Andrew Lunn 
> Sent: Friday, September 28, 2018 11:18 PM
> To: Y.b. Lu 
> Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org;
> net...@vger.kernel.org; Richard Cochran ;
> David S . Miller ; Ioana Ciocoi Radulescu
> ; Greg Kroah-Hartman
> 
> Subject: Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
> 
> > > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
> > > seems very odd. And it is not the only example.
> >
> > [Y.b. Lu] This should depended on MC firmware and APIs I think. Once the
> MC improves this, the APIs could be updated to fix this.
> 
> That is going to be hard to do. Ideally the driver should work with any
> firmware version. You don't really want to force the user to upgrade the
> driver/kernel and the firmware at the same time. So you cannot for example
> remove this pad. What you might be able to do in newer versions is actually
> use the space. But you have to be sure the current code is correctly ignoring 
> it
> and setting it to zero.

[Y.b. Lu] Thanks a lot, I think I understand now
The files dprtc* defining the APIs were provided together with MC firmware. 
They were tested working fine.
MC firmware would also consider the backward compatibility I think.
Regarding to the API files, let me remove unused code before using them, and 
keep the rest.

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


RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-28 Thread Y.b. Lu
Hi Ioana,

> -Original Message-
> From: Ioana Ciocoi Radulescu
> Sent: Friday, September 28, 2018 6:21 PM
> To: Y.b. Lu ; Andrew Lunn 
> Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org;
> net...@vger.kernel.org; Richard Cochran ;
> David S . Miller ; Greg Kroah-Hartman
> 
> Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
> 
> > -Original Message-
> > From: Y.b. Lu
> > Sent: Friday, September 28, 2018 11:04 AM
> > To: Andrew Lunn 
> > Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org;
> > net...@vger.kernel.org; Richard Cochran ;
> > David S . Miller ; Ioana Ciocoi Radulescu
> > ; Greg Kroah-Hartman
> > 
> > Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of
> > staging/
> >
[...]
> > >
> > > It seems like there is a lot of code in dprtc.c which is unused.
> > > rtc.c does
> > nothing
> > > with interrupts for example. Do you plan to make use of this extra
> > > code? Or can it be removed leaving just what is needed?
> >
> > [Y.b. Lu] Currently the ptp/rtc driver is not full-featured. The extra
> > code is being planed to be used.
> 
> Are there any interrupts associated to the real time clock module that will
> actually be used by the driver? Also, I don't think the create/destroy 
> functions
> are meant to be used by the PTP kernel driver, even though MC exposes the
> APIs for them.
> 
> Generally speaking, I think it's better to remove unused code from the current
> driver and re-add it along with the feature actually using it.

[Y.b. Lu] Yes. We need to implement these interrupts to support 
ptp_clock_event() of common ptp_clock driver.
This is mainly to support 1588 timer external signals.
I get your point, and will remove unused code before using them.

> 
> >
> > >
> > > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
> > > seems
> > very
> > > odd. And it is not the only example.
> >
> > [Y.b. Lu] This should depended on MC firmware and APIs I think. Once
> > the MC improves this, the APIs could be updated to fix this.
> 
> These structures map the command format expected by the MC firmware. I
> agree that some of the command layouts are less than inspired, but I'm not
> sure we can expect MC to "improve" them without a good reason, as this
> would break backward compatibility.
> 
> I also want to bring up the question of where the dpaa2 ptp driver should be
> located. The qoriq_ptp driver (which targets previous gen Freescale/NXP
> architectures) is located in drivers/ptp. I'm not sure if the dpaa2 ptp driver
> should be moved there as well or it's better suited for the currently proposed
> location.

[Y.b. Lu] Actually the ptp timer is to provide hw timestamping support for 
ethernet.
Ptp clock driver together with ethernet hw timestamping driver provide the 
method to support 1588 software application.
It's ok to put ptp clock driver near to ethernet driver. And most ptp clock 
drivers in kernel are together with ethernet driver.
You may see there are gianfar_ptp and dpaa_ptp before. Considering they could 
reuse the code, I created the ptp_qoriq for them to use the one driver.

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


[PATCH] Staging: rts5208: rtsx_card: Fixed multiple coding style issues

2018-09-28 Thread Maxime Desroches
Fixed multiple coding style issues

Signed-off-by: Maxime Desroches 
---
 drivers/staging/rts5208/rtsx_card.c | 96 +++--
 1 file changed, 37 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx_card.c 
b/drivers/staging/rts5208/rtsx_card.c
index d26a8e372fce..b45abbe29bc2 100644
--- a/drivers/staging/rts5208/rtsx_card.c
+++ b/drivers/staging/rts5208/rtsx_card.c
@@ -647,9 +647,8 @@ int switch_ssc_clock(struct rtsx_chip *chip, int clk)
dev_dbg(rtsx_dev(chip), "Switch SSC clock to %dMHz (cur_clk = %d)\n",
clk, chip->cur_clk);
 
-   if ((clk <= 2) || (n > max_n)) {
+   if ((clk <= 2) || (n > max_n))
return STATUS_FAIL;
-   }
 
mcu_cnt = (u8)(125 / clk + 3);
if (mcu_cnt > 7)
@@ -688,15 +687,13 @@ int switch_ssc_clock(struct rtsx_chip *chip, int clk)
}
 
retval = rtsx_send_cmd(chip, 0, WAIT_TIME);
-   if (retval < 0) {
+   if (retval < 0)
return STATUS_ERROR;
-   }
 
udelay(10);
retval = rtsx_write_register(chip, CLK_CTL, CLK_LOW_FREQ, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
chip->cur_clk = clk;
 
@@ -790,49 +787,44 @@ int switch_normal_clock(struct rtsx_chip *chip, int clk)
}
 
retval = rtsx_write_register(chip, CLK_CTL, 0xFF, CLK_LOW_FREQ);
-   if (retval) {
+   if (retval)
return retval;
-   }
if (sd_vpclk_phase_reset) {
retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
 PHASE_NOT_RESET, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
retval = rtsx_write_register(chip, SD_VPCLK1_CTL,
 PHASE_NOT_RESET, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
}
retval = rtsx_write_register(chip, CLK_DIV, 0xFF,
 (div << 4) | mcu_cnt);
-   if (retval) {
+   if (retval)
return retval;
-   }
retval = rtsx_write_register(chip, CLK_SEL, 0xFF, sel);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
if (sd_vpclk_phase_reset) {
udelay(200);
retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
 PHASE_NOT_RESET, PHASE_NOT_RESET);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
retval = rtsx_write_register(chip, SD_VPCLK1_CTL,
 PHASE_NOT_RESET, PHASE_NOT_RESET);
-   if (retval) {
+   if (retval)
return retval;
-   }
+
udelay(200);
}
retval = rtsx_write_register(chip, CLK_CTL, 0xFF, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
chip->cur_clk = clk;
 
@@ -878,9 +870,8 @@ int enable_card_clock(struct rtsx_chip *chip, u8 card)
clk_en |= MS_CLK_EN;
 
retval = rtsx_write_register(chip, CARD_CLK_EN, clk_en, clk_en);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -898,9 +889,8 @@ int disable_card_clock(struct rtsx_chip *chip, u8 card)
clk_en |= MS_CLK_EN;
 
retval = rtsx_write_register(chip, CARD_CLK_EN, clk_en, 0);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -924,9 +914,8 @@ int card_power_on(struct rtsx_chip *chip, u8 card)
rtsx_add_cmd(chip, WRITE_REG_CMD, CARD_PWR_CTL, mask, val1);
 
retval = rtsx_send_cmd(chip, 0, 100);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
udelay(chip->pmos_pwr_on_interval);
 
@@ -934,9 +923,8 @@ int card_power_on(struct rtsx_chip *chip, u8 card)
rtsx_add_cmd(chip, WRITE_REG_CMD, CARD_PWR_CTL, mask, val2);
 
retval = rtsx_send_cmd(chip, 0, 100);
-   if (retval != STATUS_SUCCESS) {
+   if (retval != STATUS_SUCCESS)
return STATUS_FAIL;
-   }
 
return STATUS_SUCCESS;
 }
@@ -955,9 +943,8 @@ int card_power_off(struct rtsx_chip *chip, u8 card)
}
 
retval = rtsx_write_register(chip, CARD_PWR_CTL, mask, val);
-   if (retval) {
+   if (retval)
return retval;
-   }
 
return STATUS_SUCCESS;
 }
@@ -969,9 +956,8 @@ int card_rw(struct scsi_cmnd *srb, struct rtsx_chip *chip,
unsigned int lun = SCSI_LUN(srb);
int i;
 
-   if (!chip->rw_card[lun]) {
+   if (!chip->rw_card[lun])

Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

2018-09-28 Thread John Whitmore
On Fri, Sep 28, 2018 at 02:35:50PM +0200, Greg KH wrote:
> On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> > The member variables AdvCoding and GreenField are unused in code so
> > have been removed from the structure and associated initialisation
> > function.
> > 
> > This is a coding style change which should have no impact on runtime
> > code execution.
> > 
> > Signed-off-by: John Whitmore 
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> >  2 files changed, 4 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h 
> > b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > index 64d5359cf7e2..83fb8f34ccbd 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
> >  
> >  struct ht_capability_ele {
> > //HT capability info
> > -   u8  AdvCoding:1;
> > u8  ChlWidth:1;
> > u8  MimoPwrSave:2;
> > -   u8  GreenField:1;
> 
> Don't these fields come from the hardware itself?  By removing them
> here, you just changed the memory layout of the structure.  Does the
> driver still work properly after this?  If you can't test it, I can't
> take this patch as it's too risky...
> 

Sorry, yes the structure looks like it should come from the hardware
but as the structure is allocated from memory I expected to find a
memcopy either to or from the hardware. Yes risky, just because I
couldn't find it don't mean the connection to hardware ain't there.

I'll lay off the risky and who knows if I keep wondering through the
driver I'll find that illusive connection.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

2018-09-28 Thread John Whitmore
On Fri, Sep 28, 2018 at 05:31:40PM +0300, Dan Carpenter wrote:
> On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> > The member variables AdvCoding and GreenField are unused in code so
> > have been removed from the structure and associated initialisation
> > function.
> > 
> > This is a coding style change which should have no impact on runtime
> > code execution.
> > 
> > Signed-off-by: John Whitmore 
> > ---
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
> >  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
> >  2 files changed, 4 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h 
> > b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > index 64d5359cf7e2..83fb8f34ccbd 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> > @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
> >  
> >  struct ht_capability_ele {
> > //HT capability info
> > -   u8  AdvCoding:1;
> > u8  ChlWidth:1;
> > u8  MimoPwrSave:2;
> > -   u8  GreenField:1;
> > u8  ShortGI20Mhz:1;
> > u8  ShortGI40Mhz:1;
> > u8  TxSTBC:1;
> 
> I feel like we discussed this before.  I'm pretty sure this comes from
> the firmware and so the format can't be changed.  When I look at
> rtllib_parse_mife_generic() then I think that "info_element" probably
> comes from the firmware.
> 
> I wouldn't want to accept this with out someone testing it.
> 
> regards,
> dan carpenter
> 

Thank you and sorry about not helping the signal to noise ratio on here.

I agree that a bit field like that and it looks like it comes from
firmware, but my question or possibly obsession was where. There are
structures inside structures, but they are all allocated from RAM.
Because of that I expected to find a memcopy from the device, or given
that the bitfield is initialised with values that it might be memcopy'd
to the device. I just couldn't find that memcopy, but that's down to
my untrained eye. I'll stumble across it in some obscure corner of the
driver.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: most: usb: add release function for DCI device

2018-09-28 Thread Christian Gromm
This patch adds the missing release function for the DCI device that frees
the container structure it is embedded in.

Signed-off-by: Christian Gromm 
---
 drivers/staging/most/usb/usb.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/usb/usb.c b/drivers/staging/most/usb/usb.c
index 7869e84..c0293d8 100644
--- a/drivers/staging/most/usb/usb.c
+++ b/drivers/staging/most/usb/usb.c
@@ -1015,6 +1015,13 @@ static const struct attribute_group *dci_attr_groups[] = 
{
NULL,
 };
 
+static void release_dci(struct device *dev)
+{
+   struct most_dci_obj *dci = to_dci_obj(dev);
+
+   kfree(dci);
+}
+
 /**
  * hdm_probe - probe function of USB device driver
  * @interface: Interface of the attached USB device
@@ -1146,6 +1153,7 @@ hdm_probe(struct usb_interface *interface, const struct 
usb_device_id *id)
mdev->dci->dev.init_name = "dci";
mdev->dci->dev.parent = >iface.dev;
mdev->dci->dev.groups = dci_attr_groups;
+   mdev->dci->dev.release = release_dci;
if (device_register(>dci->dev)) {
mutex_unlock(>io_mutex);
most_deregister_interface(>iface);
@@ -1198,7 +1206,6 @@ static void hdm_disconnect(struct usb_interface 
*interface)
cancel_work_sync(>poll_work_obj);
 
device_unregister(>dci->dev);
-   kfree(mdev->dci);
most_deregister_interface(>iface);
 
kfree(mdev->busy_urbs);
-- 
2.7.4

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


[PATCH] staging: axis-fifo: add error handling of class_create()

2018-09-28 Thread Alexey Khoroshilov
Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 drivers/staging/axis-fifo/axis-fifo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c 
b/drivers/staging/axis-fifo/axis-fifo.c
index abeee0ecc122..63c8efd1b8db 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -1089,6 +1089,8 @@ static int __init axis_fifo_init(void)
pr_info("axis-fifo driver loaded with parameters read_timeout = %i, 
write_timeout = %i\n",
read_timeout, write_timeout);
axis_fifo_driver_class = class_create(THIS_MODULE, DRIVER_NAME);
+   if (IS_ERR(axis_fifo_driver_class))
+   return PTR_ERR(axis_fifo_driver_class);
return platform_driver_register(_fifo_driver);
 }
 
-- 
2.7.4

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


Re: [PATCH RFC] mm/memory_hotplug: Introduce memory block types

2018-09-28 Thread Dave Hansen
It's really nice if these kinds of things are broken up.  First, replace
the old want_memblock parameter, then add the parameter to the
__add_page() calls.

> +/*
> + * NONE: No memory block is to be created (e.g. device memory).
> + * NORMAL:   Memory block that represents normal (boot or hotplugged) memory
> + *   (e.g. ACPI DIMMs) that should be onlined either automatically
> + *   (memhp_auto_online) or manually by user space to select a
> + *   specific zone.
> + *   Applicable to memhp_auto_online.
> + * STANDBY:  Memory block that represents standby memory that should only
> + *   be onlined on demand by user space (e.g. standby memory on
> + *   s390x), but never automatically by the kernel.
> + *   Not applicable to memhp_auto_online.
> + * PARAVIRT: Memory block that represents memory added by
> + *   paravirtualized mechanisms (e.g. hyper-v, xen) that will
> + *   always automatically get onlined. Memory will be unplugged
> + *   using ballooning, not by relying on the MOVABLE ZONE.
> + *   Not applicable to memhp_auto_online.
> + */
> +enum {
> + MEMORY_BLOCK_NONE,
> + MEMORY_BLOCK_NORMAL,
> + MEMORY_BLOCK_STANDBY,
> + MEMORY_BLOCK_PARAVIRT,
> +};

This does not seem like the best way to expose these.

STANDBY, for instance, seems to be essentially a replacement for a check
against running on s390 in userspace to implement a _typical_ s390
policy.  It seems rather weird to try to make the userspace policy
determination easier by telling userspace about the typical s390 policy
via the kernel.

As for the OOM issues, that sounds like something we need to fix by
refusing to do (or delaying) hot-add operations once we consume too much
ZONE_NORMAL from memmap[]s rather than trying to indirectly tell
userspace to hurry thing along.

So, to my eye, we need:

 +enum {
 +  MEMORY_BLOCK_NONE,
 +  MEMORY_BLOCK_STANDBY, /* the default */
 +  MEMORY_BLOCK_AUTO_ONLINE,
 +};

and we can probably collapse NONE into AUTO_ONLINE because userspace
ends up doing the same thing for both: nothing.

>  struct memory_block {
>   unsigned long start_section_nr;
>   unsigned long end_section_nr;
> @@ -34,6 +58,7 @@ struct memory_block {
>   int (*phys_callback)(struct memory_block *);
>   struct device dev;
>   int nid;/* NID for this memory block */
> + int type;   /* type of this memory block */
>  };

Shouldn't we just be creating and using an actual named enum type?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

2018-09-28 Thread Greg KH
On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote:
> Remove the member variables TxSTBC and RxSTBC as neither is used in
> code.
> 
> This is a coding style change which should not impact runtime code
> execution.

Same as before, I think this _does_ impact runtime :(

I'll stop here in the series.

thanks,

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


Re: [PATCH 01/15] staging: vboxvideo: Cleanup vbox_set_up_input_mapping()

2018-09-28 Thread Greg Kroah-Hartman
On Wed, Sep 26, 2018 at 09:41:52PM +0200, Hans de Goede wrote:
> This cleanups 2 things:
> 
> 1) The first time we loop over the crtc-s, to compare framebuffers, fb1 may
> get set to NULL by the fb1 = CRTC_FB(crtci); statement and then we call
> to_vbox_framebuffer() on it. The result of this call is only used for
> an address comparison, so we don't end up dereferencing the bad pointer,
> but still it is better to not do this.
> 
> 2) Since we already figure out the first crtc with a fb in the first loop
> and store that in fb1, there is no need to loop over the crtc-s again just
> to find the first crtc with a fb again.
> 
> Signed-off-by: Hans de Goede 

I have two patch 1/15 in this series, making it 16 patches?

Something went odd on your end, can you please resend these properly?

thanks,

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


Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

2018-09-28 Thread Greg KH
On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
> 
> This is a coding style change which should have no impact on runtime
> code execution.
> 
> Signed-off-by: John Whitmore 
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h 
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>  
>  struct ht_capability_ele {
>   //HT capability info
> - u8  AdvCoding:1;
>   u8  ChlWidth:1;
>   u8  MimoPwrSave:2;
> - u8  GreenField:1;

Don't these fields come from the hardware itself?  By removing them
here, you just changed the memory layout of the structure.  Does the
driver still work properly after this?  If you can't test it, I can't
take this patch as it's too risky...

sorry,

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


Re: [PATCH v3] staging: mt7621-mmc: Remove #if 0 blocks and fix macros in sd.c

2018-09-28 Thread Greg Kroah-Hartman
On Thu, Sep 27, 2018 at 09:10:57PM +0530, Nishad Kamdar wrote:
> This patch removes #if 0 code blocks and usages of the
> functions defined in the #if 0 code block. It removes
> the macro msdc_irq_restore() and replaces its usage
> with call to the function called in the macro definition.
> Issue found by checkpatch.

When you have to start listing all of the different things your patch
does, that's a huge hint it needs to be broken up into smaller pieces.

Please make this a patch series, only doing on "logical" change at a
time.  You are doing too many different things here all at once.

thanks

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


Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-28 Thread Andrew Lunn
> > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct seems 
> > very
> > odd. And it is not the only example.
> 
> [Y.b. Lu] This should depended on MC firmware and APIs I think. Once the MC 
> improves this, the APIs could be updated to fix this.

That is going to be hard to do. Ideally the driver should work with
any firmware version. You don't really want to force the user to
upgrade the driver/kernel and the firmware at the same time. So you
cannot for example remove this pad. What you might be able to do in
newer versions is actually use the space. But you have to be sure the
current code is correctly ignoring it and setting it to zero.

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


[PATCH RFC] mm/memory_hotplug: Introduce memory block types

2018-09-28 Thread David Hildenbrand
How to/when to online hotplugged memory is hard to manage for
distributions because different memory types are to be treated differently.
Right now, we need complicated udev rules that e.g. check if we are
running on s390x, on a physical system or on a virtualized system. But
there is also sometimes the demand to really online memory immediately
while adding in the kernel and not to wait for user space to make a
decision. And on virtualized systems there might be different
requirements, depending on "how" the memory was added (and if it will
eventually get unplugged again - DIMM vs. paravirtualized mechanisms).

On the one hand, we have physical systems where we sometimes
want to be able to unplug memory again - e.g. a DIMM - so we have to online
it to the MOVABLE zone optionally. That decision is usually made in user
space.

On the other hand, we have memory that should never be onlined
automatically, only when asked for by an administrator. Such memory only
applies to virtualized environments like s390x, where the concept of
"standby" memory exists. Memory is detected and added during boot, so it
can be onlined when requested by the admininistrator or some tooling.
Only when onlining, memory will be allocated in the hypervisor.

But then, we also have paravirtualized devices (namely xen and hyper-v
balloons), that hotplug memory that will never ever be removed from a
system right now using offline_pages/remove_memory. If at all, this memory
is logically unplugged and handed back to the hypervisor via ballooning.

For paravirtualized devices it is relevant that memory is onlined as
quickly as possible after adding - and that it is added to the NORMAL
zone. Otherwise, it could happen that too much memory in a row is added
(but not onlined), resulting in out-of-memory conditions due to the
additional memory for "struct pages" and friends. MOVABLE zone as well
as delays might be very problematic and lead to crashes (e.g. zone
imbalance).

Therefore, introduce memory block types and online memory depending on
it when adding the memory. Expose the memory type to user space, so user
space handlers can start to process only "normal" memory. Other memory
block types can be ignored. One thing less to worry about in user space.

Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Yoshinori Sato 
Cc: Rich Felker 
Cc: Dave Hansen 
Cc: Andy Lutomirski 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: "H. Peter Anvin" 
Cc: "Rafael J. Wysocki" 
Cc: Len Brown 
Cc: Greg Kroah-Hartman 
Cc: "K. Y. Srinivasan" 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: "Jérôme Glisse" 
Cc: Andrew Morton 
Cc: Mike Rapoport 
Cc: Dan Williams 
Cc: Stephen Rothwell 
Cc: Michal Hocko 
Cc: "Kirill A. Shutemov" 
Cc: David Hildenbrand 
Cc: Nicholas Piggin 
Cc: "Jonathan Neuschäfer" 
Cc: Joe Perches 
Cc: Michael Neuling 
Cc: Mauricio Faria de Oliveira 
Cc: Balbir Singh 
Cc: Rashmica Gupta 
Cc: Pavel Tatashin 
Cc: Rob Herring 
Cc: Philippe Ombredanne 
Cc: Kate Stewart 
Cc: "mike.tra...@hpe.com" 
Cc: Joonsoo Kim 
Cc: Oscar Salvador 
Cc: Mathieu Malaterre 
Signed-off-by: David Hildenbrand 
---

This patch is based on the current mm-tree, where some related
patches from me are currently residing that touched the add_memory()
functions.

 arch/ia64/mm/init.c   |  4 +-
 arch/powerpc/mm/mem.c |  4 +-
 arch/powerpc/platforms/powernv/memtrace.c |  3 +-
 arch/s390/mm/init.c   |  4 +-
 arch/sh/mm/init.c |  4 +-
 arch/x86/mm/init_32.c |  4 +-
 arch/x86/mm/init_64.c |  8 +--
 drivers/acpi/acpi_memhotplug.c|  3 +-
 drivers/base/memory.c | 63 ---
 drivers/hv/hv_balloon.c   | 33 ++--
 drivers/s390/char/sclp_cmd.c  |  3 +-
 drivers/xen/balloon.c |  2 +-
 include/linux/memory.h| 28 +-
 include/linux/memory_hotplug.h| 17 +++---
 mm/hmm.c  |  6 ++-
 mm/memory_hotplug.c   | 31 ++-
 16 files changed, 139 insertions(+), 78 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index d5e12ff1d73c..813d1d86bf95 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -646,13 +646,13 @@ mem_init (void)
 
 #ifdef CONFIG_MEMORY_HOTPLUG
 int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
-   bool want_memblock)
+   int memory_block_type)
 {
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
int ret;
 
-   ret = __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
+   ret = __add_pages(nid, start_pfn, nr_pages, altmap, 

Re: [PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning

2018-09-28 Thread Nathan Chancellor
On Fri, Sep 28, 2018 at 10:04:29AM +0100, Dave Stevenson wrote:
> Hi Nate
> 
> Thanks for the patch.
> 
> On Fri, 28 Sep 2018 at 01:53, Nathan Chancellor
>  wrote:
> >
> > Clang warns:
> >
> > drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
> > variable 'mains_freq_qmenu' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static const s64 mains_freq_qmenu[] = {
> >  ^
> > 1 warning generated.
> >
> > This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
> > macro, which is a compile time evaluation in this case. Avoid this by
> > adding mains_freq_qmenu as the imenu member of this structure, which
> > matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
> > This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
> > defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
> > definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.
> 
> There's a slight confusion betwen V4L2_CID_MPEG_VIDEO_BITRATE_MODE and
> V4L2_CID_POWER_LINE_FREQUENCY in your description.
> 
> However you're right that MMAL_CONTROL_TYPE_STD_MENU calls
> v4l2_ctrl_new_std_menu which doesn't need a menu array (it's
> v4l2_ctrl_new_int_menu that does). That means the correct fix is to
> define the max value using the relevant enum (in this case
> V4L2_CID_POWER_LINE_FREQUENCY_AUTO) and remove the array.
> 
> The same is true for V4L2_CID_MPEG_VIDEO_BITRATE_MODE - max should be
> V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, and remove the bitrate_mode_qmenu
> array.
> 
> Thanks,
>   Dave
> 

Hi Dave,

Thank you for the review, I appreciate it. I had certainly considered
that solution first butnfigured this was the more conservative change.

I will send a v2 soon with the suggested change.

Nathan

> > Link: https://github.com/ClangBuiltLinux/linux/issues/122
> > Signed-off-by: Nathan Chancellor 
> > ---
> >  drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c 
> > b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > index cff7b1e07153..a2c55cb2192a 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > @@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl 
> > v4l2_ctrls[V4L2_CTRL_COUNT] = {
> > {
> > V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
> > 0, ARRAY_SIZE(mains_freq_qmenu) - 1,
> > -   1, 1, NULL,
> > +   1, 1, mains_freq_qmenu,
> > MMAL_PARAMETER_FLICKER_AVOID,
> > _set_flicker_avoidance,
> > false
> > --
> > 2.19.0
> >
> >
> > ___
> > linux-rpi-kernel mailing list
> > linux-rpi-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4] staging: mt7621-mmc: Remove #if 0 blocks and fix macros

2018-09-28 Thread Nishad Kamdar
This patch removes #if 0 code blocks and usages of the
functions defined in the #if 0 code block. It removes
the macro msdc_irq_restore() and replaces its usage
with call to the function called in the macro definition.
Issue found by checkpatch.

Signed-off-by: Nishad Kamdar 
---
Changes in v4:
  - Removed #if 0 code blocks and usages of functions defined
in the #if 0 blocks in dbg.c and dbg.h
Changes in v3:
  - Remove the #if 0 code blocks and usages of the functions
defined in the #if 0 code block.
  - Delete msdc_irq_restore() and replace its usage directly
with call to the function being called by the macro.
Changes in v2:
  - Convert msdc_gate_clk() and msdc_ungate_clk() to inline functions.
  - Delete msdc_irq_restore(), msdc_vcore_on(), msdc_vcore_off(),
msdc_vdd_on() and msdc_vdd_off() and replace their usages directly
with calls to the function being called by these macros.
---
 drivers/staging/mt7621-mmc/dbg.c |  18 --
 drivers/staging/mt7621-mmc/dbg.h |  14 --
 drivers/staging/mt7621-mmc/sd.c  | 306 +--
 3 files changed, 2 insertions(+), 336 deletions(-)

diff --git a/drivers/staging/mt7621-mmc/dbg.c b/drivers/staging/mt7621-mmc/dbg.c
index 4fe4d2fd111e..c512cc55be3e 100644
--- a/drivers/staging/mt7621-mmc/dbg.c
+++ b/drivers/staging/mt7621-mmc/dbg.c
@@ -66,23 +66,6 @@ u32 sdio_pro_enable;   /* make sure gpt is enabled */
 u32 sdio_pro_time; /* no more than 30s */
 struct sdio_profile sdio_perfomance = {0};
 
-#if 0 /* --- chhung */
-void msdc_init_gpt(void)
-{
-   GPT_CONFIG config;
-
-   config.num  = GPT6;
-   config.mode = GPT_FREE_RUN;
-   config.clkSrc = GPT_CLK_SRC_SYS;
-   config.clkDiv = GPT_CLK_DIV_1;   /* 13MHz GPT6 */
-
-   if (GPT_Config(config) == FALSE)
-   return;
-
-   GPT_Start(GPT6);
-}
-#endif /* end of --- */
-
 u32 msdc_time_calc(u32 old_L32, u32 old_H32, u32 new_L32, u32 new_H32)
 {
u32 ret = 0;
@@ -269,7 +252,6 @@ static ssize_t msdc_debug_proc_write(struct file *file,
} else if (cmd == SD_TOOL_SDIO_PROFILE) {
if (p1 == 1) { /* enable profile */
if (gpt_enable == 0) {
-   // msdc_init_gpt(); /* --- by chhung */
gpt_enable = 1;
}
sdio_pro_enable = 1;
diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 4ab9f10dccc2..2a044c375910 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -91,23 +91,9 @@ enum msdc_dbg {
 
 extern unsigned int sd_debug_zone[4];
 #define TAG "msdc"
-#if 0 /* +++ chhung */
-#define BUG_ON(x) \
-do { \
-   if (x) { \
-   printk("[BUG] %s LINE:%d FILE:%s\n", #x, __LINE__, __FILE__); \
-   while (1)   \
-   ;   \
-   } \
-} while (0)
-#endif /* end of +++ */
 
 void msdc_debug_proc_init(void);
 
-#if 0 /* --- chhung */
-void msdc_init_gpt(void);
-extern void GPT_GetCounter64(UINT32 *cntL32, UINT32 *cntH32);
-#endif /* end of --- */
 u32 msdc_time_calc(u32 old_L32, u32 old_H32, u32 new_L32, u32 new_H32);
 void msdc_performance(u32 opcode, u32 sizes, u32 bRx, u32 ticks);
 
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index e3c1546373ba..dbf4dcb28b3c 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -72,11 +72,6 @@
 #define GPIO_PULL_DOWN  (0)
 #define GPIO_PULL_UP(1)
 
-#if 0 /* --- by chhung */
-#define MSDC_CLKSRC_REG (0xf10C)
-#define PDN_REG   (0xF110)
-#endif /* end of --- */
-
 #define DEFAULT_DEBOUNCE(8)   /* 8 cycles */
 #define DEFAULT_DTOC(40)  /* data timeout counter. 65536x40 sclk. 
*/
 
@@ -100,26 +95,6 @@ static int cd_active_low = 1;
 //#define PERI_MSDC2_PDN(17)
 //#define PERI_MSDC3_PDN(18)
 
-#if 0 /* --- by chhung */
-/* gate means clock power down */
-static int g_clk_gate = 0;
-#define msdc_gate_clock(id) \
-   do {   \
-   g_clk_gate &= ~(1 << ((id) + PERI_MSDC0_PDN));  \
-   } while (0)
-/* not like power down register. 1 means clock on. */
-#define msdc_ungate_clock(id) \
-   do {\
-   g_clk_gate |= 1 << ((id) + PERI_MSDC0_PDN); \
-   } while (0)
-
-// do we need sync object or not
-void msdc_clk_status(int *status)
-{
-   *status = g_clk_gate;
-}
-#endif /* end of --- */
-
 /* +++ by chhung */
 struct msdc_hw msdc0_hw = {
.clk_src= 0,
@@ -169,11 +144,6 @@ static void msdc_clr_fifo(struct msdc_host *host)
sdr_clr_bits(host->base + MSDC_INTEN, val); \
} while (0)
 
-#define msdc_irq_restore(val) \
-   do {\
-   

[PATCH net-next] hv_netvsc: Fix rndis_per_packet_info internal field initialization

2018-09-28 Thread Haiyang Zhang
From: Haiyang Zhang 

The RSC feature -- a bit field "internal" was added here with total
size unchanged:
struct rndis_per_packet_info {
u32 size;
u32 type:31;
u32 internal:1;
u32 ppi_offset;
};

On TX path, we put rndis msg into skb head room, which is not zeroed
before passing to us. We do not use the "internal" field in TX path,
but it may impact older hosts which use the entire 32 bits as "type".

To fix the bug, this patch sets the field "internal" to zero.

Fixes: c8e4eff4675f ("hv_netvsc: Add support for LRO/RSC in the vSwitch")
Signed-off-by: Haiyang Zhang 
---
 drivers/net/hyperv/netvsc_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index ec699741170b..005cbaa2fa3b 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -226,6 +226,7 @@ static inline void *init_ppi_data(struct rndis_message *msg,
 
ppi->size = ppi_size;
ppi->type = pkt_type;
+   ppi->internal = 0;
ppi->ppi_offset = sizeof(struct rndis_per_packet_info);
 
rndis_pkt->per_pkt_info_len += ppi_size;
-- 
2.18.0

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


Re: [PATCH 07/13] staging:rtl8192u: Remove DelayBA, PSMP and Rsvd1 - Style

2018-09-28 Thread Dan Carpenter
Yeah...  :(  All the remaining patches are similar and risky.

regards,
dan carpenter

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


Re: [PATCH 06/13] staging:rtl8192u: Remove TxSTBC and RxSTBC - Style

2018-09-28 Thread Dan Carpenter
On Wed, Sep 26, 2018 at 08:16:57PM +0100, John Whitmore wrote:
> Remove the member variables TxSTBC and RxSTBC as neither is used in
> code.
> 
> This is a coding style change which should not impact runtime code
> execution.
> 
> Signed-off-by: John Whitmore 
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 3 ---
>  2 files changed, 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h 
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 83fb8f34ccbd..52cce0dcf9a5 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -43,8 +43,6 @@ struct ht_capability_ele {
>   u8  MimoPwrSave:2;
>   u8  ShortGI20Mhz:1;
>   u8  ShortGI40Mhz:1;
> - u8  TxSTBC:1;
> - u8  RxSTBC:2;
>   u8  DelayBA:1;
>   u8  MaxAMSDUSize:1;
>   u8  DssCCk:1;

Same thing.  I suspect his is set by the firmware.

regards,
dan carpenter

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


Re: [PATCH 05/13] staging:rtl8192u: Remove AdvCoding and GreenField - Style

2018-09-28 Thread Dan Carpenter
On Wed, Sep 26, 2018 at 08:16:56PM +0100, John Whitmore wrote:
> The member variables AdvCoding and GreenField are unused in code so
> have been removed from the structure and associated initialisation
> function.
> 
> This is a coding style change which should have no impact on runtime
> code execution.
> 
> Signed-off-by: John Whitmore 
> ---
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h | 2 --
>  drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h 
> b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> index 64d5359cf7e2..83fb8f34ccbd 100644
> --- a/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> +++ b/drivers/staging/rtl8192u/ieee80211/rtl819x_HT.h
> @@ -39,10 +39,8 @@ enum ht_extension_chan_offset {
>  
>  struct ht_capability_ele {
>   //HT capability info
> - u8  AdvCoding:1;
>   u8  ChlWidth:1;
>   u8  MimoPwrSave:2;
> - u8  GreenField:1;
>   u8  ShortGI20Mhz:1;
>   u8  ShortGI40Mhz:1;
>   u8  TxSTBC:1;

I feel like we discussed this before.  I'm pretty sure this comes from
the firmware and so the format can't be changed.  When I look at
rtllib_parse_mife_generic() then I think that "info_element" probably
comes from the firmware.

I wouldn't want to accept this with out someone testing it.

regards,
dan carpenter

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


Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-28 Thread Richard Cochran
On Fri, Sep 28, 2018 at 10:20:55AM +, Ioana Ciocoi Radulescu wrote:
> Generally speaking, I think it's better to remove unused code from the current
> driver and re-add it along with the feature actually using it.

+1

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


For images 777

2018-09-28 Thread Carol

Need photos cutting out or retouching? We can help.

We are a image team and we do editing for all kinds of e-commerce photos,
portrait photos and others.

We can provide testing to check quality. You can send 1 or 2 photos to
start.

Thanks,
Carol

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


RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-28 Thread Y.b. Lu
Hi Andrew,

Thanks a lot for your comments.
Please see my comments inline.

Best regards,
Yangbo Lu

> -Original Message-
> From: Andrew Lunn 
> Sent: Thursday, September 27, 2018 9:25 PM
> To: Y.b. Lu 
> Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org;
> net...@vger.kernel.org; Richard Cochran ;
> David S . Miller ; Ioana Ciocoi Radulescu
> ; Greg Kroah-Hartman
> 
> Subject: Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
> 
> On Thu, Sep 27, 2018 at 07:12:27PM +0800, Yangbo Lu wrote:
> > This patch is to move DPAA2 PTP driver out of staging/ since the
> > dpaa2-eth had been moved out.
> >
> > Signed-off-by: Yangbo Lu 
> > ---
> >  drivers/net/ethernet/freescale/Kconfig |9 +
> >  drivers/net/ethernet/freescale/dpaa2/Kconfig   |   15
> +++
> >  drivers/net/ethernet/freescale/dpaa2/Makefile  |6 --
> >  .../ethernet/freescale/dpaa2}/dprtc-cmd.h  |0
> >  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.c   |0
> >  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.h   |0
> >  .../rtc => net/ethernet/freescale/dpaa2}/rtc.c |0
> >  .../rtc => net/ethernet/freescale/dpaa2}/rtc.h |0
> >  drivers/staging/fsl-dpaa2/Kconfig  |8 
> >  drivers/staging/fsl-dpaa2/Makefile |1 -
> >  drivers/staging/fsl-dpaa2/rtc/Makefile |7 ---
> >  11 files changed, 20 insertions(+), 26 deletions(-)  create mode
> > 100644 drivers/net/ethernet/freescale/dpaa2/Kconfig
> >  rename drivers/{staging/fsl-dpaa2/rtc =>
> > net/ethernet/freescale/dpaa2}/dprtc-cmd.h (100%)  rename
> > drivers/{staging/fsl-dpaa2/rtc =>
> > net/ethernet/freescale/dpaa2}/dprtc.c (100%)  rename
> > drivers/{staging/fsl-dpaa2/rtc =>
> > net/ethernet/freescale/dpaa2}/dprtc.h (100%)  rename
> > drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.c
> > (100%)  rename drivers/{staging/fsl-dpaa2/rtc =>
> > net/ethernet/freescale/dpaa2}/rtc.h (100%)
> 
> Hi Yangbo
> 
> Calling a ptp driver rtc.[ch] seems rather odd. Could you fixup the name,
> change it to ptp.[ch]. Also, some of the function names, and structures,
> rtc_probe->ptp_probe, rtc_remove->ptp_remove, rtc_match_id_table->
> ptp_match_id_table, etc.

[Y.b. Lu] Indeed, it's odd and confusing...
For dpaa2, all hardware resources are allocated and configured through the 
Management Complex (MC) portals.
MC abstracts most of these resources as DPAA2 objects and exposes ABIs through 
which they can be configured and controlled.
This ptp timer was named as rtc in MC firmware and APIs as you saw in dprtc.*.
So initially I wrote this driver using rtc as name.

No worries, let me change it in next version.

> 
> ptp_dpaa2_adjfreq() probably should return err, not 0.
> ptp_dpaa2_gettime() again does not return the error.
> If fact, it seems like all the main functions ignore errors.

[Y.b. Lu] Will fix the returns in next version.

> 
> kzalloc() could be changed to devm_kzalloc() to simplify the cleanup

[Y.b. Lu] Will use devm_kzalloc() in next version.

 Can
> ptp_dpaa2_caps be made const?

[Y.b. Lu] Yes. Will change it in next version.

> dpaa2_phc_index does not appear to be used.

[Y.b. Lu] It's used in dpaa2-ethtool.c for .get_ts_info interface of 
ethtool_ops.

> dev_set_drvdata(dev, NULL); is not needed.

[Y.b. Lu] Will remove it in next version.

> Can rtc_drv be made const?

[Y.b. Lu] Will use const in next version.

> Is rtc.h used by anything other than rtc.c? It seems like it can be removed.

[Y.b. Lu] Let me remove it in next version.

> 
> It seems like there is a lot of code in dprtc.c which is unused. rtc.c does 
> nothing
> with interrupts for example. Do you plan to make use of this extra code? Or
> can it be removed leaving just what is needed?

[Y.b. Lu] Currently the ptp/rtc driver is not full-featured. The extra code is 
being planed to be used.

> 
> struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct seems very
> odd. And it is not the only example.

[Y.b. Lu] This should depended on MC firmware and APIs I think. Once the MC 
improves this, the APIs could be updated to fix this.

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


RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/

2018-09-28 Thread Ioana Ciocoi Radulescu
> -Original Message-
> From: Y.b. Lu
> Sent: Friday, September 28, 2018 11:04 AM
> To: Andrew Lunn 
> Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org;
> net...@vger.kernel.org; Richard Cochran ;
> David S . Miller ; Ioana Ciocoi Radulescu
> ; Greg Kroah-Hartman
> 
> Subject: RE: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
> 
> Hi Andrew,
> 
> Thanks a lot for your comments.
> Please see my comments inline.
> 
> Best regards,
> Yangbo Lu
> 
> > -Original Message-
> > From: Andrew Lunn 
> > Sent: Thursday, September 27, 2018 9:25 PM
> > To: Y.b. Lu 
> > Cc: linux-ker...@vger.kernel.org; de...@driverdev.osuosl.org;
> > net...@vger.kernel.org; Richard Cochran ;
> > David S . Miller ; Ioana Ciocoi Radulescu
> > ; Greg Kroah-Hartman
> > 
> > Subject: Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of
> staging/
> >
> > On Thu, Sep 27, 2018 at 07:12:27PM +0800, Yangbo Lu wrote:
> > > This patch is to move DPAA2 PTP driver out of staging/ since the
> > > dpaa2-eth had been moved out.
> > >
> > > Signed-off-by: Yangbo Lu 
> > > ---
> > >  drivers/net/ethernet/freescale/Kconfig |9 +
> > >  drivers/net/ethernet/freescale/dpaa2/Kconfig   |   15
> > +++
> > >  drivers/net/ethernet/freescale/dpaa2/Makefile  |6 --
> > >  .../ethernet/freescale/dpaa2}/dprtc-cmd.h  |0
> > >  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.c   |0
> > >  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.h   |0
> > >  .../rtc => net/ethernet/freescale/dpaa2}/rtc.c |0
> > >  .../rtc => net/ethernet/freescale/dpaa2}/rtc.h |0
> > >  drivers/staging/fsl-dpaa2/Kconfig  |8 
> > >  drivers/staging/fsl-dpaa2/Makefile |1 -
> > >  drivers/staging/fsl-dpaa2/rtc/Makefile |7 ---
> > >  11 files changed, 20 insertions(+), 26 deletions(-)  create mode
> > > 100644 drivers/net/ethernet/freescale/dpaa2/Kconfig
> > >  rename drivers/{staging/fsl-dpaa2/rtc =>
> > > net/ethernet/freescale/dpaa2}/dprtc-cmd.h (100%)  rename
> > > drivers/{staging/fsl-dpaa2/rtc =>
> > > net/ethernet/freescale/dpaa2}/dprtc.c (100%)  rename
> > > drivers/{staging/fsl-dpaa2/rtc =>
> > > net/ethernet/freescale/dpaa2}/dprtc.h (100%)  rename
> > > drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.c
> > > (100%)  rename drivers/{staging/fsl-dpaa2/rtc =>
> > > net/ethernet/freescale/dpaa2}/rtc.h (100%)
> >
> > Hi Yangbo
> >
> > Calling a ptp driver rtc.[ch] seems rather odd. Could you fixup the name,
> > change it to ptp.[ch]. Also, some of the function names, and structures,
> > rtc_probe->ptp_probe, rtc_remove->ptp_remove, rtc_match_id_table->
> > ptp_match_id_table, etc.
> 
> [Y.b. Lu] Indeed, it's odd and confusing...
> For dpaa2, all hardware resources are allocated and configured through the
> Management Complex (MC) portals.
> MC abstracts most of these resources as DPAA2 objects and exposes ABIs
> through which they can be configured and controlled.
> This ptp timer was named as rtc in MC firmware and APIs as you saw in
> dprtc.*.
> So initially I wrote this driver using rtc as name.
> 
> No worries, let me change it in next version.
> 
> >
> > ptp_dpaa2_adjfreq() probably should return err, not 0.
> > ptp_dpaa2_gettime() again does not return the error.
> > If fact, it seems like all the main functions ignore errors.
> 
> [Y.b. Lu] Will fix the returns in next version.
> 
> >
> > kzalloc() could be changed to devm_kzalloc() to simplify the cleanup
> 
> [Y.b. Lu] Will use devm_kzalloc() in next version.
> 
>  Can
> > ptp_dpaa2_caps be made const?
> 
> [Y.b. Lu] Yes. Will change it in next version.
> 
> > dpaa2_phc_index does not appear to be used.
> 
> [Y.b. Lu] It's used in dpaa2-ethtool.c for .get_ts_info interface of
> ethtool_ops.
> 
> > dev_set_drvdata(dev, NULL); is not needed.
> 
> [Y.b. Lu] Will remove it in next version.
> 
> > Can rtc_drv be made const?
> 
> [Y.b. Lu] Will use const in next version.
> 
> > Is rtc.h used by anything other than rtc.c? It seems like it can be removed.
> 
> [Y.b. Lu] Let me remove it in next version.
> 
> >
> > It seems like there is a lot of code in dprtc.c which is unused. rtc.c does
> nothing
> > with interrupts for example. Do you plan to make use of this extra code? Or
> > can it be removed leaving just what is needed?
> 
> [Y.b. Lu] Currently the ptp/rtc driver is not full-featured. The extra code is
> being planed to be used.

Are there any interrupts associated to the real time clock module that will
actually be used by the driver? Also, I don't think the create/destroy functions
are meant to be used by the PTP kernel driver, even though MC exposes the
APIs for them.

Generally speaking, I think it's better to remove unused code from the current
driver and re-add it along with the feature actually using it.
 
> 
> >
> > struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct seems
> very
> > odd. And it 

Re: [PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning

2018-09-28 Thread Dave Stevenson
Hi Nate

Thanks for the patch.

On Fri, 28 Sep 2018 at 01:53, Nathan Chancellor
 wrote:
>
> Clang warns:
>
> drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
> variable 'mains_freq_qmenu' is not needed and will not be emitted
> [-Wunneeded-internal-declaration]
> static const s64 mains_freq_qmenu[] = {
>  ^
> 1 warning generated.
>
> This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
> macro, which is a compile time evaluation in this case. Avoid this by
> adding mains_freq_qmenu as the imenu member of this structure, which
> matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
> This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
> defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
> definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.

There's a slight confusion betwen V4L2_CID_MPEG_VIDEO_BITRATE_MODE and
V4L2_CID_POWER_LINE_FREQUENCY in your description.

However you're right that MMAL_CONTROL_TYPE_STD_MENU calls
v4l2_ctrl_new_std_menu which doesn't need a menu array (it's
v4l2_ctrl_new_int_menu that does). That means the correct fix is to
define the max value using the relevant enum (in this case
V4L2_CID_POWER_LINE_FREQUENCY_AUTO) and remove the array.

The same is true for V4L2_CID_MPEG_VIDEO_BITRATE_MODE - max should be
V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, and remove the bitrate_mode_qmenu
array.

Thanks,
  Dave

> Link: https://github.com/ClangBuiltLinux/linux/issues/122
> Signed-off-by: Nathan Chancellor 
> ---
>  drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c 
> b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> index cff7b1e07153..a2c55cb2192a 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> @@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl 
> v4l2_ctrls[V4L2_CTRL_COUNT] = {
> {
> V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
> 0, ARRAY_SIZE(mains_freq_qmenu) - 1,
> -   1, 1, NULL,
> +   1, 1, mains_freq_qmenu,
> MMAL_PARAMETER_FLICKER_AVOID,
> _set_flicker_avoidance,
> false
> --
> 2.19.0
>
>
> ___
> linux-rpi-kernel mailing list
> linux-rpi-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

2018-09-28 Thread Aymen Qader
On Fri, Sep 28, 2018 at 11:13:25AM +0300, Dan Carpenter wrote:
> You might want to try running Smatch on your patches.  This is the
> second one where maybe the results would have been interesting.
> 
> git clone http://repo.or.cz/w/smatch.git
> cd smatch
> make
> cd ~/kernel/src/
> ~/smatch/smatch_scripts/kchecker drivers/staging/ks7010/ks_hostif.c
> 
> This patch introduced a problem, but the earlier one fixed a potential
> problem in rtl8723bs:
> 
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:1274 OnAssocReq() error: 
> uninitialized symbol 'ie_len'.
> 
> Unfortunately, Smatch missed the potential NULL dereference in
> OnAssocReq()...  It wouldn't be that hard to fix...
> 
> regards,
> dan carpenter

Oh thanks, I've not used Smatch before--that's really helpful, I'm going
to use that a lot in the future.

On Fri, Sep 28, 2018 at 11:16:01AM +0300, Dan Carpenter wrote:
> Btw, if you have the cross function DB built then Smatch says that the
> NULL check can be removed here no problem.
> 
> $ smdb hostif_data_request
> [ snip ]
> drivers/staging/ks7010/ks_wlan_net.c |   ks_wlan_start_xmit |  
> hostif_data_request |  PARAM_VALUE |  1 | skb | s64min-(-1),1-s64max
> 
> regards,
> dan carpenter
> 

Again, thanks very much! That's really cool, Smatch looks like a great
tool, I wish I found it before!

On Fri, Sep 28, 2018 at 11:22:39AM +0300, Dan Carpenter wrote:
> On Thu, Sep 27, 2018 at 07:04:43PM +0100, Aymen Qader wrote:
> > Retraction: in hindsight I see that with the current usage of this
> > function, there is already a check for the socket buffer so this check
> > is unnecessary. However, I'm not sure if it's considered good practice
> > to keep this check anyway--in any case, ENOMEM isn't the right error
> > to return.
> 
> When we find inconsistent NULL checks then we fix it to make sense.
> Generally, we prefer a minimal style, with no extra code for future
> proofing.  (The future seldom goes the way you expect and those extra
> NULL checks would be easy to add back).

Hm I understand, so if a parameter can currently never be null there's
no need to add unnecessary checks for in case that changes in the
future. I'll be sure to keep that in mind from now on.

> 
> So, yes, do remove the NULL check but also fix the indenting while
> you're at it.
> 
> Take your time to write patches.  I write them, then I sit on them over
> night then I send them in the morning.  It means that sometimes other
> people have already sent it but that's fine.  If you have to redo a
> patch then don't send the v2 patch on the same day.  v2 patches are
> stressful and you imagine that everyone is waiting for you to send it
> or something.  We are not waiting for you.  We don't care if you wait
> until next week to send these...
> 
> So when you write a v2 patch wait until the next day to send it.  Then
> you will be calm when you review it.
> 
> regards,
> dan carpenter
>  

Thank you very much for all of these tips, I really appreciate you
taking the time. I'm definitely going to try and apply this philosophy
in the future, I think I've been too impulsive with my patches so far.
I've been thinking "oh no, I made a mistake--I need to quickly send a
v2!" and then end up making even more mistakes, so I completely
understand what you mean.

Thanks again.

Kind regards,
Aymen Qader

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


Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

2018-09-28 Thread Dan Carpenter
On Thu, Sep 27, 2018 at 07:04:43PM +0100, Aymen Qader wrote:
> Retraction: in hindsight I see that with the current usage of this
> function, there is already a check for the socket buffer so this check
> is unnecessary. However, I'm not sure if it's considered good practice
> to keep this check anyway--in any case, ENOMEM isn't the right error
> to return.

When we find inconsistent NULL checks then we fix it to make sense.
Generally, we prefer a minimal style, with no extra code for future
proofing.  (The future seldom goes the way you expect and those extra
NULL checks would be easy to add back).

So, yes, do remove the NULL check but also fix the indenting while
you're at it.

Take your time to write patches.  I write them, then I sit on them over
night then I send them in the morning.  It means that sometimes other
people have already sent it but that's fine.  If you have to redo a
patch then don't send the v2 patch on the same day.  v2 patches are
stressful and you imagine that everyone is waiting for you to send it
or something.  We are not waiting for you.  We don't care if you wait
until next week to send these...

So when you write a v2 patch wait until the next day to send it.  Then
you will be calm when you review it.

regards,
dan carpenter

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


Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

2018-09-28 Thread Dan Carpenter
Btw, if you have the cross function DB built then Smatch says that the
NULL check can be removed here no problem.

$ smdb hostif_data_request
[ snip ]
drivers/staging/ks7010/ks_wlan_net.c |   ks_wlan_start_xmit |  
hostif_data_request |  PARAM_VALUE |  1 | skb | s64min-(-1),1-s64max

regards,
dan carpenter

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


Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

2018-09-28 Thread Dan Carpenter
You might want to try running Smatch on your patches.  This is the
second one where maybe the results would have been interesting.

git clone http://repo.or.cz/w/smatch.git
cd smatch
make
cd ~/kernel/src/
~/smatch/smatch_scripts/kchecker drivers/staging/ks7010/ks_hostif.c

This patch introduced a problem, but the earlier one fixed a potential
problem in rtl8723bs:

drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:1274 OnAssocReq() error: 
uninitialized symbol 'ie_len'.

Unfortunately, Smatch missed the potential NULL dereference in
OnAssocReq()...  It wouldn't be that hard to fix...

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