From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
Date: Fri, 13 Dec 2019 09:49:55 +0100
Subject: [PATCH v6 081/102] x86: Add a generic Intel GPIO driver
In-Reply-To: <20191206213936.v6.81.I364be7c3fc47205b9e5f90030ae246c8e85cb9f1@changeid>
References: <20191206213936.v6.81.I364be7c3fc47205b9e5f90030ae246c8e85cb9f1@changeid>,
 <20191207044315.51770-1-sjg@chromium.org>
Message-ID: <OF42650F52.E3ED63A1-ONC12584CF.002FCC75-C12584CF.00308407@br-automation.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon, Bin,

> +static int intel_gpio_direction_output(struct udevice *dev, uint offset,
> +				       int value)
> +{
> +	struct udevice *pinctrl = dev_get_parent(dev);
> +	uint config_offset = intel_pinctrl_get_config_reg_addr(pinctrl, offset);
> +
> +	pcr_clrsetbits32(dev, config_offset,

I think we should pass 'pinctrl' instead of 'dev' here.
As far as I understand the code the function pcr_clrsetbits32 expects a pinctrl
device with a p2sb parent.

> +			 PAD_CFG0_MODE_MASK | PAD_CFG0_RX_STATE |
> +				  PAD_CFG0_TX_DISABLE,

We also need to clear the bit PAD_CFG0_TX_STATE here.
Otherwise if a gpio is set to high once it can never be set to low again.

> +			 PAD_CFG0_MODE_GPIO | PAD_CFG0_RX_DISABLE |
> +				  (value ? PAD_CFG0_TX_STATE : 0));
> +
> +	return 0;
> +}

regards, Wolfgang