Re: [PATCH v6 4/5] drm/bridge: anx7625: add HDCP support
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
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 147 ++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 36 ++
> 2 files changed, 183 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
> b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 8c514b46d361..b424a570effa 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -633,6 +633,150 @@ static int anx7625_dpi_config(struct anx7625_data *ctx)
> return ret;
> }
>
> +static int anx7625_aux_dpcd_read(struct anx7625_data *ctx,
> +u8 addrh, u8 addrm, u8 addrl,
> +u8 len, u8 *buf)
> +{
> + struct device *dev = >client->dev;
> + int ret;
> + u8 cmd;
> +
> + if (len > MAX_DPCD_BUFFER_SIZE) {
> + DRM_DEV_ERROR(dev, "exceed aux buffer len.\n");
> + return -E2BIG;
> + }
> +
> + cmd = ((len - 1) << 4) | 0x09;
> +
> + /* Set command and length */
> + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> + AP_AUX_COMMAND, cmd);
> +
> + /* Set aux access address */
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_ADDR_7_0, addrl);
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_ADDR_15_8, addrm);
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_ADDR_19_16, addrh);
> +
> + /* Enable aux access */
> + ret |= anx7625_write_or(ctx, ctx->i2c.rx_p0_client,
> + AP_AUX_CTRL_STATUS, AP_AUX_CTRL_OP_EN);
> +
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "cannot access aux related register.\n");
> + return -EIO;
> + }
> +
> + usleep_range(2000, 2100);
> +
> + ret = wait_aux_op_finish(ctx);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "aux IO error: wait aux op finish.\n");
> + return ret;
> + }
> +
> + ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
> +AP_AUX_BUFF_START, len, buf);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "read dpcd register failed\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int anx7625_read_flash_status(struct anx7625_data *ctx)
> +{
> + return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client, R_RAM_CTRL);
> +}
> +
> +static int anx7625_hdcp_key_probe(struct anx7625_data *ctx)
> +{
> + int ret, val;
> + struct device *dev = >client->dev;
> + u8 ident[32];
> +
> + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> + FLASH_ADDR_HIGH, 0x91);
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +FLASH_ADDR_LOW, 0xA0);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "IO error : set key flash address.\n");
> + return ret;
> + }
> +
> + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> + FLASH_LEN_HIGH, (FLASH_BUF_LEN - 1) >> 8);
> + ret |= anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> +FLASH_LEN_LOW, (FLASH_BUF_LEN - 1) & 0xFF);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "IO error : set key flash len.\n");
> + return ret;
> + }
> +
> + ret = anx7625_reg_write(ctx, ctx->i2c.rx_p0_client,
> + R_FLASH_RW_CTRL, FLASH_READ);
> + ret |= readx_poll_timeout(anx7625_read_flash_status,
> + ctx, val,
> + ((val & FLASH_DONE) || (val < 0)),
> + 2000,
> + 2000 * 150);
> + if (ret) {
> + DRM_DEV_ERROR(dev, "flash read access fail!\n");
> + return -EIO;
> + }
> +
> + ret = anx7625_reg_block_read(ctx, ctx->i2c.rx_p0_client,
> +FLASH_BUF_BASE_ADDR,
> +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 = >client->dev;
> +
> + ret = anx7625_hdcp_key_probe(ctx);
> + if (ret) {
> +
Re: [PATCH] staging: gasket: remove it from the kernel
On 25.03.21 15:52, Greg KH wrote: > On Thu, Mar 25, 2021 at 03:46:10PM +0100, Jan Kiszka wrote: >> On 15.03.21 17:10, Rob Springer wrote: >>> Acked-by: Rob Springer >>> >>> >>> On Mon, Mar 15, 2021 at 8:44 AM wrote: From: Greg Kroah-Hartman As none of the proposed things that need to be changed in the gasket drivers have ever been done, and there has not been any forward progress to get this out of staging, it seems totally abandonded so remove the code entirely so that people do not spend their time doing tiny cleanups for code that will never get out of staging. If this code is actually being used, it can be reverted simply and then cleaned up properly, but as it is abandoned, let's just get rid of it. Cc: Rob Springer Cc: Todd Poynor Cc: Ben Chan Cc: Richard Yeh Signed-off-by: Greg Kroah-Hartman >> >> OK, so is there a plan of the HW vendor to improve the user experience >> for this hardware? Is there a different software architecture in sight >> which will not need a kernel driver? > > What hardware vendor makes this thing? What systems require it? And > why can't you use UIO instead? > >> Just wondering loudly while fiddling with dkms packages and starring at >> the code diffs between what was removed here and what I still have to >> install manually from remote sources. > > Where are the remote sources for this thing and why didn't they ever get > synced into the kernel tree? > Very good questions, and I'm curious to learn if someone in CC can answer them. I was just starting to play with this thing, using Google's binary Debian repo. But that is not... optimal. Even more when thinking beyond a try-out stage. Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gasket: remove it from the kernel
On 15.03.21 17:10, Rob Springer wrote: > Acked-by: Rob Springer > > > On Mon, Mar 15, 2021 at 8:44 AM wrote: >> >> From: Greg Kroah-Hartman >> >> As none of the proposed things that need to be changed in the gasket >> drivers have ever been done, and there has not been any forward progress >> to get this out of staging, it seems totally abandonded so remove the >> code entirely so that people do not spend their time doing tiny cleanups >> for code that will never get out of staging. >> >> If this code is actually being used, it can be reverted simply and then >> cleaned up properly, but as it is abandoned, let's just get rid of it. >> >> Cc: Rob Springer >> Cc: Todd Poynor >> Cc: Ben Chan >> Cc: Richard Yeh >> Signed-off-by: Greg Kroah-Hartman OK, so is there a plan of the HW vendor to improve the user experience for this hardware? Is there a different software architecture in sight which will not need a kernel driver? Just wondering loudly while fiddling with dkms packages and starring at the code diffs between what was removed here and what I still have to install manually from remote sources. Thanks, Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gasket: remove it from the kernel
On Thu, Mar 25, 2021 at 03:46:10PM +0100, Jan Kiszka wrote: > On 15.03.21 17:10, Rob Springer wrote: > > Acked-by: Rob Springer > > > > > > On Mon, Mar 15, 2021 at 8:44 AM wrote: > >> > >> From: Greg Kroah-Hartman > >> > >> As none of the proposed things that need to be changed in the gasket > >> drivers have ever been done, and there has not been any forward progress > >> to get this out of staging, it seems totally abandonded so remove the > >> code entirely so that people do not spend their time doing tiny cleanups > >> for code that will never get out of staging. > >> > >> If this code is actually being used, it can be reverted simply and then > >> cleaned up properly, but as it is abandoned, let's just get rid of it. > >> > >> Cc: Rob Springer > >> Cc: Todd Poynor > >> Cc: Ben Chan > >> Cc: Richard Yeh > >> Signed-off-by: Greg Kroah-Hartman > > OK, so is there a plan of the HW vendor to improve the user experience > for this hardware? Is there a different software architecture in sight > which will not need a kernel driver? What hardware vendor makes this thing? What systems require it? And why can't you use UIO instead? > Just wondering loudly while fiddling with dkms packages and starring at > the code diffs between what was removed here and what I still have to > install manually from remote sources. Where are the remote sources for this thing and why didn't they ever get synced into the kernel tree? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 17/21] spmi: hisi-spmi-controller: move driver from staging
Em Fri, 5 Feb 2021 16:19:47 -0600
Rob Herring escreveu:
> On Tue, Jan 19, 2021 at 05:10:43PM +0100, Mauro Carvalho Chehab wrote:
> > The Hisilicon 6421v600 SPMI driver is ready for mainstream.
> >
> > So, move it from staging.
> >
> > Signed-off-by: Mauro Carvalho Chehab
> > ---
> > .../spmi/hisilicon,hisi-spmi-controller.yaml | 75
> > MAINTAINERS | 7 +
> > drivers/spmi/Kconfig | 9 +
> > drivers/spmi/Makefile | 1 +
> > drivers/spmi/hisi-spmi-controller.c | 358 ++
> > drivers/staging/hikey9xx/Kconfig | 11 -
> > drivers/staging/hikey9xx/Makefile | 1 -
> > .../staging/hikey9xx/hisi-spmi-controller.c | 358 --
> > .../hisilicon,hisi-spmi-controller.yaml | 75
> > 9 files changed, 450 insertions(+), 445 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> > create mode 100644 drivers/spmi/hisi-spmi-controller.c
> > delete mode 100644 drivers/staging/hikey9xx/hisi-spmi-controller.c
> > delete mode 100644
> > drivers/staging/hikey9xx/hisilicon,hisi-spmi-controller.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> >
> > b/Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> > new file mode 100644
> > index ..21f68a9c2df1
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/spmi/hisilicon,hisi-spmi-controller.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: HiSilicon SPMI controller
> > +
> > +maintainers:
> > + - Mauro Carvalho Chehab
> > +
> > +description: |
> > + The HiSilicon SPMI BUS controller is found on some Kirin-based designs.
> > + It is a MIPI System Power Management (SPMI) controller.
> > +
> > + The PMIC part is provided by
> > + drivers/staging/hikey9xx/hisilicon,hi6421-spmi-pmic.yaml.
> > +
> > +properties:
> > + $nodename:
> > +pattern: "spmi@[0-9a-f]"
> > +
> > + compatible:
> > +const: hisilicon,kirin970-spmi-controller
>
> '-controller' is kind of redundant.
Ok. Will drop it.
>
> > +
> > + reg:
> > +maxItems: 1
> > +
>
> > + "#address-cells":
> > +const: 2
> > +
> > + "#size-cells":
> > +const: 0
>
> These 2 are covered by spmi.yaml
Ok.
>
> > +
> > + spmi-channel:
> > +description: |
> > + number of the Kirin 970 SPMI channel where the SPMI devices are
> > connected.
>
> Common to SPMI? If not, needs a vendor prefix.
That's an interesting question. My understanding is that this is not
vendor-specific, but maybe Stephen can give us more details.
The spmi.h header calls it "nr", and documents it at include/linux/spmi.h
as:
/**
* struct spmi_controller - interface to the SPMI master controller
* @dev:Driver model representation of the device.
* @nr: board-specific number identifier for this controller/bus
* @cmd:sends a non-data command sequence on the SPMI bus.
* @read_cmd: sends a register read command sequence on the SPMI bus.
* @write_cmd: sends a register write command sequence on the SPMI bus.
*/
There, it says that this is "board-specific number identifier".
Yet, as the SPMI is a serial bus with up to 4 masters (controller), I
suspect that the idea is to associate it with the master ID.
This is used on boards with multiple SoCs. See, for instance, slide 5 of:
https://www.mipi.org/sites/default/files/Bangalore-Qualcomm-SPMI-1.0-Multi-master-Verification.pdf
However, it is hard to know for sure, as no drivers use it, except by
Hikey 970 controller:
$ grep "\b\->nr\b" $(git grep -l spmi.h)
drivers/spmi/spmi.c:ida_simple_remove(_ida, ctrl->nr);
drivers/spmi/spmi.c:dev_set_name(>dev, "%d-%02x", ctrl->nr,
sdev->usid);
drivers/spmi/spmi.c:ctrl->nr = id;
drivers/spmi/spmi.c:ctrl->nr, >dev);
drivers/staging/hikey9xx/hisi-spmi-controller.c:ctrl->nr =
spmi_controller->channel;
>
> Type? Range of values?
The SPMI core defines it as "unsigned int". So, I would use:
$ref: /schemas/types.yaml#/definitions/uint32
as a type.
At the driver, this is used to calculate the channel offset with:
static int spmi_write_cmd(...) {
u32 chnl_ofst = SPMI_CHANNEL_OFFSET * spmi_controller->channel;
...
writel((u32 __force)cpu_to_be32(data),
spmi_controller->base + chnl_ofst +
SPMI_APB_SPMI_WDATA0_BASE_ADDR +
SPMI_PER_DATAREG_BYTE * i);
...
}
As on both spmi.h
Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced
On Thu, Mar 25, 2021 at 07:03:39PM +0800, Jian Dong wrote: > On Thu, 25 Mar 2021 11:29:06 +0100 > Greg KH wrote: > > > On Thu, Mar 25, 2021 at 06:19:26PM +0800, Jian Dong wrote: > > > From: Jian Dong > > > > > > fixes coccicheck Error: > > > > > > drivers/staging/greybus/bootrom.c:301:41-45: ERROR: > > > fw is NULL but dereferenced. > > > > > > if procedure goto label directly, ret will be nefative, so the fw > > > is NULL and the if(condition) end with dereferenced fw. let's fix > > > it. > > > > Why is this all indented a space? > this maybe caused by my terminal, I will take notice next time. > > > > > > > > Signed-off-by: Jian Dong > > > --- > > > drivers/staging/greybus/bootrom.c | 8 > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/greybus/bootrom.c > > > b/drivers/staging/greybus/bootrom.c index a8efb86..0439efa 100644 > > > --- a/drivers/staging/greybus/bootrom.c > > > +++ b/drivers/staging/greybus/bootrom.c > > > @@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct > > > gb_operation *op) struct gb_bootrom_get_firmware_response > > > *firmware_response; struct device *dev = > > > >connection->bundle->dev; unsigned int offset, size; > > > - enum next_request_type next_request; > > > + enum next_request_type next_request = > > > NEXT_REQ_GET_FIRMWARE; int ret = 0; > > > > > > /* Disable timeouts */ > > > @@ -298,10 +298,10 @@ static int gb_bootrom_get_firmware(struct > > > gb_operation *op) > > > queue_work: > > > /* Refresh timeout */ > > > - if (!ret && (offset + size == fw->size)) > > > - next_request = NEXT_REQ_READY_TO_BOOT; > > > - else > > > + if (!!ret) > > > > That is hard to understand, please make this more obvious. > > > if (A && B) else (!A || !B) > > So, when ret is NON-ZERO, set next_request as GET_FIRMWARE, else set > READ_TO_BOOT. but if second express is flase, next_request still > need be set as GET_FIRMWARE, So, I initialze it as GET_FIRMWARE. My point is: if (!!ret) is odd, and is the same thing as: if (ret) correct? And the latter is the common kernel style, no need to be complex when you do not need to. Anyway, others have pointed out why this is incorrect, no need for further discussion. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced
On Thu, 25 Mar 2021 11:29:06 +0100 Greg KH wrote: > On Thu, Mar 25, 2021 at 06:19:26PM +0800, Jian Dong wrote: > > From: Jian Dong > > > > fixes coccicheck Error: > > > > drivers/staging/greybus/bootrom.c:301:41-45: ERROR: > > fw is NULL but dereferenced. > > > > if procedure goto label directly, ret will be nefative, so the fw > > is NULL and the if(condition) end with dereferenced fw. let's fix > > it. > > Why is this all indented a space? this maybe caused by my terminal, I will take notice next time. > > > > > Signed-off-by: Jian Dong > > --- > > drivers/staging/greybus/bootrom.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/greybus/bootrom.c > > b/drivers/staging/greybus/bootrom.c index a8efb86..0439efa 100644 > > --- a/drivers/staging/greybus/bootrom.c > > +++ b/drivers/staging/greybus/bootrom.c > > @@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct > > gb_operation *op) struct gb_bootrom_get_firmware_response > > *firmware_response; struct device *dev = > > >connection->bundle->dev; unsigned int offset, size; > > - enum next_request_type next_request; > > + enum next_request_type next_request = > > NEXT_REQ_GET_FIRMWARE; int ret = 0; > > > > /* Disable timeouts */ > > @@ -298,10 +298,10 @@ static int gb_bootrom_get_firmware(struct > > gb_operation *op) > > queue_work: > > /* Refresh timeout */ > > - if (!ret && (offset + size == fw->size)) > > - next_request = NEXT_REQ_READY_TO_BOOT; > > - else > > + if (!!ret) > > That is hard to understand, please make this more obvious. > if (A && B) else (!A || !B) So, when ret is NON-ZERO, set next_request as GET_FIRMWARE, else set READ_TO_BOOT. but if second express is flase, next_request still need be set as GET_FIRMWARE, So, I initialze it as GET_FIRMWARE. this is will keep consistent with the origin conditional express: both ret is ZERO and second express TRUE, then set as READ_TO_BOOT, else set as GET_FIRMWARE. > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced
On 25-03-21, 18:19, Jian Dong wrote: > From: Jian Dong > > fixes coccicheck Error: > > drivers/staging/greybus/bootrom.c:301:41-45: ERROR: > fw is NULL but dereferenced. > > if procedure goto label directly, ret will be nefative, so the fw is NULL > and the if(condition) end with dereferenced fw. let's fix it. No, fw is accessed only for !ret case. > Signed-off-by: Jian Dong > --- > drivers/staging/greybus/bootrom.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/greybus/bootrom.c > b/drivers/staging/greybus/bootrom.c > index a8efb86..0439efa 100644 > --- a/drivers/staging/greybus/bootrom.c > +++ b/drivers/staging/greybus/bootrom.c > @@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct gb_operation > *op) > struct gb_bootrom_get_firmware_response *firmware_response; > struct device *dev = >connection->bundle->dev; > unsigned int offset, size; > - enum next_request_type next_request; > + enum next_request_type next_request = NEXT_REQ_GET_FIRMWARE; > int ret = 0; > > /* Disable timeouts */ > @@ -298,10 +298,10 @@ static int gb_bootrom_get_firmware(struct gb_operation > *op) > > queue_work: > /* Refresh timeout */ > - if (!ret && (offset + size == fw->size)) > - next_request = NEXT_REQ_READY_TO_BOOT; > - else > + if (!!ret) > next_request = NEXT_REQ_GET_FIRMWARE; > + else if (offset + size == fw->size) > + next_request = NEXT_REQ_READY_TO_BOOT; > > gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS); The code is fine AFAICT, the coccicheck is buggy as it is detecting a bug here. -- viresh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced
The commit description is not clear but this patch doesn't change how the code works, it just silences a static checker false positive. Just ignore the false positive. Always just ignore static checkers when they are wrong. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: fix fw is NULL but dereferenced
From: Jian Dong fixes coccicheck Error: drivers/staging/greybus/bootrom.c:301:41-45: ERROR: fw is NULL but dereferenced. if procedure goto label directly, ret will be nefative, so the fw is NULL and the if(condition) end with dereferenced fw. let's fix it. Signed-off-by: Jian Dong --- drivers/staging/greybus/bootrom.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index a8efb86..0439efa 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op) struct gb_bootrom_get_firmware_response *firmware_response; struct device *dev = >connection->bundle->dev; unsigned int offset, size; - enum next_request_type next_request; + enum next_request_type next_request = NEXT_REQ_GET_FIRMWARE; int ret = 0; /* Disable timeouts */ @@ -298,10 +298,10 @@ static int gb_bootrom_get_firmware(struct gb_operation *op) queue_work: /* Refresh timeout */ - if (!ret && (offset + size == fw->size)) - next_request = NEXT_REQ_READY_TO_BOOT; - else + if (!!ret) next_request = NEXT_REQ_GET_FIRMWARE; + else if (offset + size == fw->size) + next_request = NEXT_REQ_READY_TO_BOOT; gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: fix fw is NULL but dereferenced
On Thu, Mar 25, 2021 at 06:19:26PM +0800, Jian Dong wrote: > From: Jian Dong > > fixes coccicheck Error: > > drivers/staging/greybus/bootrom.c:301:41-45: ERROR: > fw is NULL but dereferenced. > > if procedure goto label directly, ret will be nefative, so the fw is NULL > and the if(condition) end with dereferenced fw. let's fix it. Why is this all indented a space? > > Signed-off-by: Jian Dong > --- > drivers/staging/greybus/bootrom.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/greybus/bootrom.c > b/drivers/staging/greybus/bootrom.c > index a8efb86..0439efa 100644 > --- a/drivers/staging/greybus/bootrom.c > +++ b/drivers/staging/greybus/bootrom.c > @@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct gb_operation > *op) > struct gb_bootrom_get_firmware_response *firmware_response; > struct device *dev = >connection->bundle->dev; > unsigned int offset, size; > - enum next_request_type next_request; > + enum next_request_type next_request = NEXT_REQ_GET_FIRMWARE; > int ret = 0; > > /* Disable timeouts */ > @@ -298,10 +298,10 @@ static int gb_bootrom_get_firmware(struct gb_operation > *op) > > queue_work: > /* Refresh timeout */ > - if (!ret && (offset + size == fw->size)) > - next_request = NEXT_REQ_READY_TO_BOOT; > - else > + if (!!ret) That is hard to understand, please make this more obvious. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
