From: Kishon Vijay Abraham I <kishon@ti.com>
To: u-boot@lists.denx.de
Subject: [PATCH v2 03/17] drivers: reset: Handle gracefully NULL pointers
Date: Tue, 4 May 2021 15:31:59 +0530 [thread overview]
Message-ID: <dbcd2f5f-cf24-7b40-8834-2e50d8ae01df@ti.com> (raw)
In-Reply-To: <CAPnjgZ01ALM_r5cm3PWjRjqUPH0SZ0of-CuoiKREmWwOQUM0gw@mail.gmail.com>
Hi Simon,
On 15/04/21 1:08 am, Simon Glass wrote:
> Hi Kishon,
>
> On Mon, 5 Apr 2021 at 11:28, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>
>> From: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>
>> The reset framework provides devm_reset_control_get_optional()
>> which can return NULL (not an error case). So all the other reset_ops
>> should handle NULL gracefully. Prepare the way for a managed reset
>> API by handling NULL pointers without crashing nor failing.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> drivers/reset/reset-uclass.c | 30 +++++++++++++++++-------------
>> 1 file changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
>> index 071c389ca0..98304bc0ee 100644
>> --- a/drivers/reset/reset-uclass.c
>> +++ b/drivers/reset/reset-uclass.c
>> @@ -13,9 +13,12 @@
>> #include <dm/devres.h>
>> #include <dm/lists.h>
>>
>> -static inline struct reset_ops *reset_dev_ops(struct udevice *dev)
>> +struct reset_ops nop_reset_ops = {
>> +};
>> +
>> +static inline struct reset_ops *reset_dev_ops(struct reset_ctl *r)
>> {
>> - return (struct reset_ops *)dev->driver->ops;
>> + return r ? (struct reset_ops *)r->dev->driver->ops : &nop_reset_ops;
>
> This behaviour still seems odd to me. Why do you have a reset driver
> with no ops? That is not allowed.
Okay. I'll re-work this patch and post a fresh series.
Thanks
Kishon
>
>> }
>>
>> static int reset_of_xlate_default(struct reset_ctl *reset_ctl,
>> @@ -54,9 +57,10 @@ static int reset_get_by_index_tail(int ret, ofnode node,
>> debug("%s %d\n", ofnode_get_name(args->node), args->args[0]);
>> return ret;
>> }
>> - ops = reset_dev_ops(dev_reset);
>>
>> reset_ctl->dev = dev_reset;
>> + ops = reset_dev_ops(reset_ctl);
>> +
>> if (ops->of_xlate)
>> ret = ops->of_xlate(reset_ctl, args);
>> else
>> @@ -162,29 +166,29 @@ int reset_get_by_name(struct udevice *dev, const char *name,
>>
>> int reset_request(struct reset_ctl *reset_ctl)
>> {
>> - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
>> + struct reset_ops *ops = reset_dev_ops(reset_ctl);
>>
>> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>>
>> - return ops->request(reset_ctl);
>> + return ops->request ? ops->request(reset_ctl) : 0;
>
> Can you check this first and return -ENOSYS ? E.g.
>
> if (!ops->request)
> return -ENOSYS;
>
> return ops->request(reset_ctl);
>
> Same below
>
>> }
>>
>> int reset_free(struct reset_ctl *reset_ctl)
>> {
>> - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
>> + struct reset_ops *ops = reset_dev_ops(reset_ctl);
>>
>> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>>
>> - return ops->rfree(reset_ctl);
>> + return ops->rfree ? ops->rfree(reset_ctl) : 0;
>> }
>>
>> int reset_assert(struct reset_ctl *reset_ctl)
>> {
>> - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
>> + struct reset_ops *ops = reset_dev_ops(reset_ctl);
>>
>> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>>
>> - return ops->rst_assert(reset_ctl);
>> + return ops->rst_assert ? ops->rst_assert(reset_ctl) : 0;
>> }
>>
>> int reset_assert_bulk(struct reset_ctl_bulk *bulk)
>> @@ -202,11 +206,11 @@ int reset_assert_bulk(struct reset_ctl_bulk *bulk)
>>
>> int reset_deassert(struct reset_ctl *reset_ctl)
>> {
>> - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
>> + struct reset_ops *ops = reset_dev_ops(reset_ctl);
>>
>> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>>
>> - return ops->rst_deassert(reset_ctl);
>> + return ops->rst_deassert ? ops->rst_deassert(reset_ctl) : 0;
>> }
>>
>> int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
>> @@ -224,11 +228,11 @@ int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
>>
>> int reset_status(struct reset_ctl *reset_ctl)
>> {
>> - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
>> + struct reset_ops *ops = reset_dev_ops(reset_ctl);
>>
>> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>>
>> - return ops->rst_status(reset_ctl);
>> + return ops->rst_status ? ops->rst_status(reset_ctl) : 0;
>> }
>>
>> int reset_release_all(struct reset_ctl *reset_ctl, int count)
>> --
>> 2.17.1
>>
>
> Regards,
> Simon
>
next prev parent reply other threads:[~2021-05-04 10:01 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-05 10:28 [PATCH v2 00/17] TI/Cadence: Add Sierra/Torrent SERDES driver Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 01/17] dm: core: Add helper to compare node names Kishon Vijay Abraham I
2021-04-14 19:37 ` Simon Glass
2021-04-05 10:28 ` [PATCH v2 02/17] dm: test: Add test case to check node name ignoring unit address Kishon Vijay Abraham I
2021-04-14 19:37 ` Simon Glass
2021-04-05 10:28 ` [PATCH v2 03/17] drivers: reset: Handle gracefully NULL pointers Kishon Vijay Abraham I
2021-04-14 19:38 ` Simon Glass
2021-05-04 10:01 ` Kishon Vijay Abraham I [this message]
2021-04-05 10:28 ` [PATCH v2 04/17] dt-bindings: phy: Add definitions for additional phy types Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 05/17] phy: cadence: Add driver for Sierra PHY Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 06/17] phy: cadence: Add driver for Torrent SERDES Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 07/17] phy: ti: j721e-wiz: Add support for WIZ module present in TI J721E SoC Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 08/17] usb: cdns3: cdns3-ti: Fix clk_get_by_name() to get the correct name Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 09/17] board: ti: j721e: Add support for probing and configuring Torrent serdes on J7200 Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 10/17] ARM: dts: k3-j721e: Add the entries required for USB3 support on USB0 Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 11/17] arm: dts: k3-j7200-main: Add DT node for torrent serdes Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 12/17] arm: dts: k3-j7200-common-proc-board: Enable SERDES DT Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 13/17] arm: dts: k3-j7200-common-proc-board-u-boot: Add u-boot tags for torrent serdes Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 14/17] configs: j721e_evm_a72: Enable the drivers required for the USB3 support Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 15/17] configs: j7200_evm_a72_defconfig: Add config for torrent serdes and common clock framework Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 16/17] env: ti: j721e-evm: Add env variable to power on & reset QSGMII PHY in J7200 EVM Kishon Vijay Abraham I
2021-04-05 10:28 ` [PATCH v2 17/17] configs: j7200_evm_a72: Enhance bootcmd to configure ethernet PHY Kishon Vijay Abraham I
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=dbcd2f5f-cf24-7b40-8834-2e50d8ae01df@ti.com \
--to=kishon@ti.com \
--cc=u-boot@lists.denx.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox