linux-kernel - Re: [PATCH v0 1/2] gpio: syscon: Add gpio-syscon for rk3328

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: <2356128.gufDZceMBS@phil>
Date:   Tue, 08 May 2018 13:49:46 +0200
From:   Heiko Stuebner <heiko@...ech.de>
To:     linux-rockchip@...ts.infradead.org
Cc:     djw@...hip.com.cn, linux-gpio@...r.kernel.org,
        Wayne Chou <zxf@...hip.com.cn>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v0 1/2] gpio: syscon: Add gpio-syscon for rk3328

Hi Levin,

Am Dienstag, 8. Mai 2018, 04:48:23 CEST schrieb djw@...hip.com.cn:
> From: Levin Du <djw@...hip.com.cn>
> 
> In Rockchip RK3328 Soc, there's a output only gpio pin labeled
> `gpiomut_pmuio_iout`, which can be set by bit[1] of GRF_SOC_CON10.
> (bit[0] controls the enable state of the pin and defaults to enabled.)
> 
> This pin is used by the roc-rk3328-cc board to switch sdmmc io signal
> voltage between 1.8V and 3.3V, which is essential to the SD card UHS
> support.
> 
> Signed-off-by: Levin Du <djw@...hip.com.cn>

Thanks for investigating that special pin.

Please also add a devicetree-binding document under
Documentation/devicetree/bindings/gpio.

And I do have some more suggestions below.


> ---
> 
>  drivers/gpio/gpio-syscon.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> index 537cec7..b69f65f 100644
> --- a/drivers/gpio/gpio-syscon.c
> +++ b/drivers/gpio/gpio-syscon.c
> @@ -135,6 +135,34 @@ static const struct syscon_gpio_data clps711x_mctrl_gpio = {
>  	.dat_bit_offset	= 0x40 * 8 + 8,
>  };
>  
> +static void rockchip_gpio_set(struct gpio_chip *chip, unsigned int offset,
> +			      int val)
> +{
> +	struct syscon_gpio_priv *priv = gpiochip_get_data(chip);
> +	unsigned int offs;
> +	u8 bit;
> +	u32 data;
> +	int ret;
> +
> +	offs = priv->dreg_offset + priv->data->dat_bit_offset + offset;
> +	bit = offs % SYSCON_REG_BITS;
> +	data = (val ? BIT(bit) : 0) | BIT(bit + 16);
> +	ret = regmap_write(priv->syscon,
> +			   (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE,
> +			   data);
> +	if (ret < 0)
> +		dev_err(chip->parent, "gpio write failed ret(%d)\n", ret);
> +}
> +
> +static const struct syscon_gpio_data rk3328_gpio_syscon10 = {
> +	/* Rockchip RK3328 GRF_SOC_CON10 Bits 0-1 */
> +	.compatible	= "rockchip,rk3328-grf",

please drop the compatible above, include the attached patch before
this one and follow the things I'll will outline in the devicetree patch
shortly :-)

Patch for getting the syscon frome the parent is compile-tested only
so please double-check that I didn't mess up anything.


> +	.flags		= GPIO_SYSCON_FEAT_OUT,
> +	.bit_count	= 2,
> +	.dat_bit_offset	= 0x0428 * 8,
> +	.set		= rockchip_gpio_set,
> +};
> +
>  #define KEYSTONE_LOCK_BIT BIT(0)
>  
>  static void keystone_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
> @@ -175,6 +203,10 @@ static const struct of_device_id syscon_gpio_ids[] = {
>  		.compatible	= "ti,keystone-dsp-gpio",
>  		.data		= &keystone_dsp_gpio,
>  	},
> +	{
> +		.compatible	= "rockchip,rk3328-gpio-syscon10",

rockchip,rk3328-gpio-mute [the naming from the TRM] could
be a more suitable naming?


Heiko
View attachment "0001-gpio-syscon-allow-fetching-syscon-from-parent-node.patch" of type "text/x-patch" (1196 bytes)