Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v7 1/3] MIPS: Loongson64: Add Loongson-2K1000 reset
 platform driver
To:     Qing Zhang <zhangqing@loongson.cn>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Rob Herring <robh+dt@kernel.org>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc:     devicetree@vger.kernel.org, linux-mips@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20210701030137.10566-1-zhangqing@loongson.cn>
From:   Sergei Shtylyov <sergei.shtylyov@gmail.com>
Organization: Brain-dead Software
Message-ID: <3e48b6b3-62e4-8a15-6c73-e2f35c07b842@gmail.com>
Date:   Thu, 1 Jul 2021 12:04:17 +0300
User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <20210701030137.10566-1-zhangqing@loongson.cn>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello!

On 01.07.2021 6:01, Qing Zhang wrote:

> Add power management register operations to support reboot and poweroff.
> 
> Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
[...]
> diff --git a/drivers/platform/mips/ls2k-reset.c b/drivers/platform/mips/ls2k-reset.c
> new file mode 100644
> index 000000000000..286d47bf6964
> --- /dev/null
> +++ b/drivers/platform/mips/ls2k-reset.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2021, Qing Zhang <zhangqing@loongson.cn>
> + *  Loongson-2K1000 reset support
> + */
> +
> +#include <linux/of_address.h>
> +#include <linux/pm.h>
> +#include <asm/reboot.h>
> +
> +#define	PM1_STS		0x0c /* Power Management1 Status Register */
                                                         ^ space missing?

> +#define	PM1_CNT		0x14 /* Power Management 1 Control Register */
> +#define	RST_CNT		0x30 /* Reset Control Register */
> +
> +static void __iomem *base;
> +
> +static void ls2k_restart(char *command)
> +{
> +	writel(0x1, (void *)base + RST_CNT);
> +}
> +
> +static void ls2k_poweroff(void)
> +{
> +	/* Clear */
> +	writel((readl((void *)base + PM1_STS) & 0xffffffff), (void *)base + PM1_STS);

    Are the pointer casts really necessary above?

> +	/* Sleep Enable | Soft Off*/
> +	writel(GENMASK(12, 10)|BIT(13), (void *)base + PM1_CNT);

    Spaces around that | are needed.

[...]

MBR, Sergei