linux-kernel - Re: [PATCH v2 13/14] reset: starfive: Add StarFive JH7110 reset driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <62d63104-e392-b002-717d-682193d21c27@starfivetech.com>
Date:   Tue, 22 Nov 2022 13:55:14 +0800
From:   Hal Feng <hal.feng@...rfivetech.com>
To:     Emil Renner Berthing <emil.renner.berthing@...onical.com>
CC:     "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        Conor Dooley <conor@...nel.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>,
        "Michael Turquette" <mturquette@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 13/14] reset: starfive: Add StarFive JH7110 reset
 driver

On Sat, 19 Nov 2022 01:14:50 +0800, Emil Renner Berthing wrote:
> On Fri, 18 Nov 2022 at 02:06, Hal Feng <hal.feng@...rfivetech.com> wrote:
>> diff --git a/drivers/reset/starfive/reset-starfive-jh7110.c b/drivers/reset/starfive/reset-starfive-jh7110.c
>> new file mode 100644
>> index 000000000000..00f3b4ecfb02
>> --- /dev/null
>> +++ b/drivers/reset/starfive/reset-starfive-jh7110.c
>> @@ -0,0 +1,67 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Reset driver for the StarFive JH7110 SoC
>> + *
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + * Copyright (C) 2022 Hal Feng <hal.feng@...rfivetech.com>
>> + */
>> +
>> +#include <linux/auxiliary_bus.h>
>> +
>> +#include "reset-starfive-jh71x0.h"
>> +
>> +#include <dt-bindings/reset/starfive-jh7110.h>
>> +
>> +static int jh7110_reset_probe(struct auxiliary_device *adev,
>> +                             const struct auxiliary_device_id *id)
>> +{
>> +       struct reset_info *info = (struct reset_info *)(id->driver_data);
>> +       void __iomem *base = dev_get_drvdata(adev->dev.parent);
>> +
>> +       if (!info || !base)
>> +               return -ENODEV;
>> +
>> +       return reset_starfive_jh71x0_register(&adev->dev, adev->dev.parent->of_node,
>> +                                             base + info->assert_offset,
>> +                                             base + info->status_offset,
>> +                                             info->asserted,
>> +                                             info->nr_resets,
>> +                                             false);
>> +}
>> +
>> +static const struct reset_info jh7110_sys_info = {
>> +       .nr_resets = JH7110_SYSRST_END,
>> +       .assert_offset = 0x2F8,
>> +       .status_offset = 0x308,
>> +       .asserted = NULL,
>> +};
>> +
>> +static const struct reset_info jh7110_aon_info = {
>> +       .nr_resets = JH7110_AONRST_END,
>> +       .assert_offset = 0x38,
>> +       .status_offset = 0x3C,
>> +       .asserted = NULL,
>> +};
> 
> It doesn't seem like syscrg, aoncrg or stgcrg have any inverted lines.
> Do you know if any other CRGs do? If not you can just leave out the
> .asserted member and always pass NULL.

All JH7110 CRGs don't have inverted lines, and it could be modified as
what you said.

> 
>> +static const struct auxiliary_device_id jh7110_reset_ids[] = {
>> +       {
>> +               .name = "clk_starfive_jh71x0.reset-sys",
>> +               .driver_data = (kernel_ulong_t)&jh7110_sys_info,
>> +       },
>> +       {
>> +               .name = "clk_starfive_jh71x0.reset-aon",
>> +               .driver_data = (kernel_ulong_t)&jh7110_aon_info,
>> +       },
>> +       { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(auxiliary, jh7110_reset_ids);
>> +
>> +static struct auxiliary_driver jh7110_reset_driver = {
>> +       .probe          = jh7110_reset_probe,
>> +       .id_table       = jh7110_reset_ids,
>> +};
>> +module_auxiliary_driver(jh7110_reset_driver);
>> +
>> +MODULE_DESCRIPTION("StarFive JH7110 Reset Driver");
>> +MODULE_AUTHOR("Hal Feng <hal.feng@...rfivetech.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/reset/starfive/reset-starfive-jh71x0.h b/drivers/reset/starfive/reset-starfive-jh71x0.h
>> index e6b27110de48..63a94ee1b395 100644
>> --- a/drivers/reset/starfive/reset-starfive-jh71x0.h
>> +++ b/drivers/reset/starfive/reset-starfive-jh71x0.h
>> @@ -6,6 +6,13 @@
>>  #ifndef __RESET_STARFIVE_JH71X0_H
>>  #define __RESET_STARFIVE_JH71X0_H
>>
>> +struct reset_info {
>> +       unsigned int nr_resets;
>> +       unsigned int assert_offset;
>> +       unsigned int status_offset;
>> +       const u32 *asserted;
>> +};
> 
> This struct isn't used outside of reset-starfive-jh7110.c, so no need
> to define it in this header.
> Also consider calling it jh7110_reset_info so it blends in with the
> functions defined in that file.

Maybe it can be used in JH7100 audio reset?

Best regards,
Hal

> 
>>  int reset_starfive_jh71x0_register(struct device *dev, struct device_node *of_node,
>>                                    void __iomem *assert, void __iomem *status,
>>                                    const u32 *asserted, unsigned int nr_resets,