All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: angelogioacchino.delregno@somainline.org
Cc: dri-devel@lists.freedesktop.org
Subject: [bug report] drm/panel: Add BOE BF060Y8M-AJ0 5.99" AMOLED panel driver
Date: Thu, 18 Nov 2021 17:59:37 +0300	[thread overview]
Message-ID: <20211118145937.GA20921@kili> (raw)

Hello AngeloGioacchino Del Regno,

The patch a19125a28112: "drm/panel: Add BOE BF060Y8M-AJ0 5.99" AMOLED
panel driver" from Sep 1, 2021, leads to the following (unpublished)
Smatch static checker warning:

drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c:317 boe_bf060y8m_aj0_init_vregs() info: return a literal instead of 'ret'
drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c:322 boe_bf060y8m_aj0_init_vregs() info: return a literal instead of 'ret'
drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c:327 boe_bf060y8m_aj0_init_vregs() info: return a literal instead of 'ret'
drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c:332 boe_bf060y8m_aj0_init_vregs() info: return a literal instead of 'ret'
drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c:338 boe_bf060y8m_aj0_init_vregs() info: return a literal instead of 'ret'

drivers/gpu/drm/panel/panel-boe-bf060y8m-aj0.c
    296 static int boe_bf060y8m_aj0_init_vregs(struct boe_bf060y8m_aj0 *boe,
    297                                        struct device *dev)
    298 {
    299         struct regulator *vreg;
    300         int ret;
    301 
    302         boe->vregs[BF060Y8M_VREG_VCC].supply = "vcc";
    303         boe->vregs[BF060Y8M_VREG_VDDIO].supply = "vddio";
    304         boe->vregs[BF060Y8M_VREG_VCI].supply = "vci";
    305         boe->vregs[BF060Y8M_VREG_EL_VDD].supply = "elvdd";
    306         boe->vregs[BF060Y8M_VREG_EL_VSS].supply = "elvss";
    307         ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(boe->vregs),
    308                                       boe->vregs);
    309         if (ret < 0) {
    310                 dev_err(dev, "Failed to get regulators: %d\n", ret);
    311                 return ret;
    312         }
    313 
    314         vreg = boe->vregs[BF060Y8M_VREG_VCC].consumer;
    315         ret = regulator_is_supported_voltage(vreg, 2700000, 3600000);
    316         if (!ret)
                ^^^^^^^^^

--> 317                 return ret;

It's more clear to "return 0;" instead of "return ret;"

The documentation for regulator_is_supported_voltage() says it returns
bool but actually it returns negatives for if regulator_get_voltage()
fails and one for if it's supported and zero for when it's not.

These if (!ret) seem wrong.

    318 
    319         vreg = boe->vregs[BF060Y8M_VREG_VDDIO].consumer;
    320         ret = regulator_is_supported_voltage(vreg, 1620000, 1980000);
    321         if (!ret)
                ^^^^^^^^^

    322                 return ret;
    323 
    324         vreg = boe->vregs[BF060Y8M_VREG_VCI].consumer;
    325         ret = regulator_is_supported_voltage(vreg, 2600000, 3600000);
    326         if (!ret)
                ^^^^^^^^^

    327                 return ret;
    328 
    329         vreg = boe->vregs[BF060Y8M_VREG_EL_VDD].consumer;
    330         ret = regulator_is_supported_voltage(vreg, 4400000, 4800000);
    331         if (!ret)
                ^^^^^^^^^

    332                 return ret;
    333 
    334         /* ELVSS is negative: -5.00V to -1.40V */
    335         vreg = boe->vregs[BF060Y8M_VREG_EL_VSS].consumer;
    336         ret = regulator_is_supported_voltage(vreg, 1400000, 5000000);
    337         if (!ret)
                ^^^^^^^^

    338                 return ret;
    339 
    340         /*
    341          * Set min/max rated current, known only for VCI and VDDIO and,
    342          * in case of failure, just go on gracefully, as this step is not
    343          * guaranteed to succeed on all regulator HW but do a debug print
    344          * to inform the developer during debugging.
    345          * In any case, these two supplies are also optional, so they may
    346          * be fixed-regulator which, at the time of writing, does not
    347          * support fake current limiting.
    348          */
    349         vreg = boe->vregs[BF060Y8M_VREG_VDDIO].consumer;
    350         ret = regulator_set_current_limit(vreg, 1500, 2500);
    351         if (ret)
    352                 dev_dbg(dev, "Current limit cannot be set on %s: %d\n",
    353                         boe->vregs[1].supply, ret);
    354 
    355         vreg = boe->vregs[BF060Y8M_VREG_VCI].consumer;
    356         ret = regulator_set_current_limit(vreg, 20000, 40000);
    357         if (ret)
    358                 dev_dbg(dev, "Current limit cannot be set on %s: %d\n",
    359                         boe->vregs[2].supply, ret);
    360 
    361         return 0;
    362 }

regards,
dan carpenter

                 reply	other threads:[~2021-11-18 15:00 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211118145937.GA20921@kili \
    --to=dan.carpenter@oracle.com \
    --cc=angelogioacchino.delregno@somainline.org \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.