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;
Date:   Wed, 21 Apr 2021 16:52:12 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Pho Tran <photranvan0712@gmail.com>
Cc:     gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, Hung.Nguyen@silabs.com,
        Tung.Pham@silabs.com, Pho Tran <pho.tran@silabs.com>
Subject: Re: [PATCH v9] USB: serial: cp210x: Add support for GPIOs on CP2108
Message-ID: <YIA8HD0S6C+x5ZC9@hovoldconsulting.com>
References: <20210408103607.2077-1-photranvan0712@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210408103607.2077-1-photranvan0712@gmail.com>
Precedence: bulk

On Thu, Apr 08, 2021 at 05:36:07PM +0700, Pho Tran wrote:
> From: Pho Tran <pho.tran@silabs.com>
> 
> Similar to other CP210x devices, GPIO interfaces (gpiochip) should be
> supported for CP2108.
 
> +/*
> + * Quad Port Config definitions
> + * Refer to https://www.silabs.com/documents/public/application-notes/an978-cp210x-usb-to-uart-api-specification.pdf
> + * for more information.
> + * CP210X_VENDOR_SPECIFIC, CP210X_GET_PORTCONFIG call reads these 0x49 bytes
> + * on a CP2108 chip.
> + * CP2108 Quad Port State structure(used in Quad Port Config structure)
> + */
> +struct cp210x_quad_port_state {
> +	__le16 gpio_mode_PB0;
> +	__le16 gpio_mode_PB1;
> +	__le16 gpio_mode_PB2;
> +	__le16 gpio_mode_PB3;
> +	__le16 gpio_mode_PB4;
> +
> +
> +	__le16 gpio_lowpower_PB0;
> +	__le16 gpio_lowpower_PB1;
> +	__le16 gpio_lowpower_PB2;
> +	__le16 gpio_lowpower_PB3;
> +	__le16 gpio_lowpower_PB4;
> +
> +	__le16 gpio_latch_PB0;
> +	__le16 gpio_latch_PB1;
> +	__le16 gpio_latch_PB2;
> +	__le16 gpio_latch_PB3;
> +	__le16 gpio_latch_PB4;
> +};
> +
> +// Cp2108 Quad Port Config structure
> +struct cp210x_quad_port_config {
> +	struct cp210x_quad_port_state reset_state;
> +	struct cp210x_quad_port_state suspend_state;
> +	u8 ipdelay_IFC[4];
> +	u8 enhancedfxn_IFC[4];
> +	u8 enhancedfxn_device;
> +	u8 extclkfreq[4];
> +} __packed;

One more thing; I noticed that the layout of the other port-config
structures do not match the ones used by your library API, which is what
the above pdf documents (e.g. they have additional padding).

Did you verify that the above layout is actually correct? And did you
try changing the pin functions in EEPROM and make sure that your code
handles it as expected?

Is there any corresponding document for the actual device protocol?

Johan