Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 13 Dec 2022 14:53:45 +0100 (CET)
From:   =?UTF-8?Q?Sven_Z=C3=BChlsdorf?= <sven.zuehlsdorf@vigem.de>
To:     Bjorn Helgaas <helgaas@kernel.org>
cc:     Lars Povlsen <lars.povlsen@microchip.com>,
        Rishi Gupta <gupt21@gmail.com>, Jiri Kosina <jikos@kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        UNGLinuxDriver <UNGLinuxDriver@microchip.com>,
        linux-i2c@vger.kernel.org, linux-input@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Takayuki 'January June' Suwa <jjsuwa_sys3175@yahoo.co.jp>
Subject: Re: [PATCH] HID: mcp2221: Fix GPIO output handling
In-Reply-To: <20221129153933.GA716319@bhelgaas>
Message-ID: <477db3e4-ff7d-ac9d-9786-76aba31defc3@vigem.de>
References: <20221129153933.GA716319@bhelgaas>
Content-Type: multipart/mixed; BOUNDARY="8323329-1626605204-1670602151=:61397"
Content-ID: <377e7981-3f29-9f98-490b-89af369751b4@sven-laptop>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cHhMYW1sSHh4N2JQZUNNc0ZFWHUxakJWS0MzbXdaeDJJUkJIdVBOL094RFhv?=
 =?utf-8?B?SVJVZUZ0VlV2eXY1RFVZSkVodEw2ZXFPb3pJdm13WHR1WFBlMGViOUJjelI5?=
 =?utf-8?B?WTZBaGpPc3pnN2FnS0tqSjJwRlNOSWk4a3FrQTZUQnJXVzNzKzdONWpvMlJR?=
 =?utf-8?B?UmJVT29SSE8vdzdHeWRWZ3NNd09PVCszOENxRkx6Mzh4djV2V0ViVVJXbE5p?=
 =?utf-8?B?VEhEa3lrOFR4UjhMWmczRmJVUVJCYWFpbnNuY0wvV2tGSUxldVBlYlZmMVFi?=
 =?utf-8?B?UkV1SGNMR2ZaZDVwaG5PbTlHd0FzdTY0b2JsOGJlUGQvTVFHTTUzaEZpMTdZ?=
 =?utf-8?B?Zlc5NXFtWnNoZEJtUzBlMHZqR1VaQjFMTGtjVWxZcWduUVBBUWw3cDd2SkZu?=
 =?utf-8?B?TXBVNGxmY20xd0ZhSE5qcmVwbmtIbytQZTFEekJTSnpFbmNHbTVRVTRWanBL?=
 =?utf-8?B?OExScEZoNXpYeGduK05BeDVaQktjc2ZMZFpDN01yMzVYM1NITWk4a1VSaExl?=
 =?utf-8?B?clUwOVdUVU1zTlQvRm1aek5qN3FhZzUxSFViZWU0M2xKbHd5c2Z6a25GWE5o?=
 =?utf-8?B?RkFWdkJ1RzE1bmNWVTJzRWc5REowWVFPdFJ4Rjl3UE5VWWZ2UDE0cWU1dEc2?=
 =?utf-8?B?UVN2aEs5QmFVU2Q5NkxNcWo0MlNNRStsUjIrUndoU2hyczA0NDVSNTZzUU1p?=
 =?utf-8?B?LzM2bGg0V2R2R0s1WmtDeXhIOGRUc2pkMDdDeTlqSDJseFN5Y2FwcDR4WWYx?=
 =?utf-8?B?ZHAxWDF5ZXQvQ3JTRG8xZXpsUVk4cXh5eDRJcktyeEd5WG1QZXBSK0h2clNP?=
 =?utf-8?B?TDQyVjJ3Y0pEcmVXTDJKRFVFcXV4SXBLMk1HVEwwV3h1dmdtWXBINlFCZHc2?=
 =?utf-8?B?VU9DakxlMTlLTmJESCtZK2hOWkdaYWJ0eTZaM3NyVXZaclYwNVdLbVlkT00x?=
 =?utf-8?B?aXptMTNuUFh1SUM2dXFmekVLNitLYVc4WE1hcmVLU1R0K0ErMWZ2Nll0SnFM?=
 =?utf-8?B?bmx6d1NXVVdRalBYNTZKQjRJRUVQbGdwN3RwdklLK3pEbzVDaGFObGIzMDNv?=
 =?utf-8?B?RTkrRkF5MFFaMVNHa29kR1hVRG93eDFGeDRJVkJsRWU3OG5xTWtLV3dlOG9o?=
 =?utf-8?B?dGdDM0Y0V2RaWVo0b2JDWFdob0FQQVlUQjJPa2w2QTY4Z29SZjZDay8rU2dO?=
 =?utf-8?B?azVBallSOTg3RjVVUFBPbkt4emRXQi9zTnRpUTgwY0w3anhNdlVid0tMbjRO?=
 =?utf-8?B?NElSeDJlRkN1RkNwWFU3b05CSmcvUFpDSEpjN0U2WUhxY2dtWDYzTVRCYTVQ?=
 =?utf-8?B?YkpLcUhYRk92SzdyTXo5a2NGYVhYV21jVVgwbENKY3dpbkE3WE8yQW5Wc1My?=
 =?utf-8?B?L2hMdGlpSlRHQUtnSnVlbUhwSmc4ejBBeVRFTHBPU2xvOFE5Vjg0bEdJZ2VZ?=
 =?utf-8?B?Tk15Z2FvVXBNbXFCVTJDZHd3ZmQrb0EyVHE1M1pYWlY1RCtYY0lJaGIxZG1Y?=
 =?utf-8?B?THBZamZPS1hsWWhtWmMreUhTSktLbzRIUWJFRUpSUG1lYVV2eGZ3QXBYemhJ?=
 =?utf-8?B?Zk9qNndHUUlqeVk0eks4ZFdNelFuaTZZRUdwVllmMFpxK2JhQUJHOEVJbnpi?=
 =?utf-8?B?UXpLcW9XajhQMmoyQUR5UzdZbWVPN3E4M1ZCdDF4MHd2a2VTdjlSanBCOU1V?=
 =?utf-8?B?TkJFa1dlNERMNFN1TUNKaUdiQ09WL1FoNXlLUzZ5VEs5ZWJ4U3drRStKVmJY?=
 =?utf-8?B?WDczQUVoR2M0THhFb1Vhb2Zra3V2UVRsTmpBYUt5aEo3bENaS3J1TWZySkkw?=
 =?utf-8?B?N05vcjJaQjlObUs0Ukd2by9kckFIRGZZWXVYTzdDY3RNTHhGOW5Bbys2eVFl?=
 =?utf-8?B?NHVONGp4RTc1dUZld1YrMklyNU1NeExycmM2dWNpOGt0RkN2eklzVFhRYlVK?=
 =?utf-8?B?Yzc3NEgya3lYYVFNL0xBYU1VWU8rNktNa3k2TVVOZzE0VHA0SUpzdXR6MW1t?=
 =?utf-8?B?a0JnUjEvNjdtQTVNOTIvbnRQU1RUZTVxbFloakhSMGYxdHZJZjNZTUdBb1Ax?=
 =?utf-8?B?K283Vlh3Zk41cE1TNXhvcU51aDRJWEdNckdtbXMvVUlUZEdpb0tsbHpyTzJM?=
 =?utf-8?B?Q2I3Rld2SGh3a2Z0dCtNWHdUclJCbWs1VU5rcmZPU0dkYkJHVG80cHJYd1ZO?=
 =?utf-8?B?VWc9PQ==?=
X-OriginatorOrg: vigem.de
X-MS-Exchange-CrossTenant-Network-Message-Id: 865fae80-7d86-4d39-b7aa-08dadd117dc2
X-MS-Exchange-CrossTenant-AuthSource: DU0P190MB1930.EURP190.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2022 13:54:02.2160
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: dc3cc44e-876e-4b98-9250-42df44be4933
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: pvsyFf6CnY/yiZGH1zKtuQbnlaDU7E1NYCpCE2oE0pa1pd/rLlNIO/tnus7p7byiRvQz/C5aGqj8JrcRv0klTFvtCDzReSTP7okEcxQhrkU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GV1P190MB1921
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

--8323329-1626605204-1670602151=:61397
Content-Type: text/plain; CHARSET=UTF-8; format=flowed
Content-Transfer-Encoding: 8BIT
Content-ID: <a7027915-b330-bfde-3074-542c83bd80ba@sven-laptop>

On Tue, 29 Nov 2022, Bjorn Helgaas wrote:
> [+cc Takayuki, thanks for your report!]
>
> On Wed, Nov 04, 2020 at 11:02:23PM +0100, Lars Povlsen wrote:
>> The mcp2221 driver GPIO output handling has has several issues.
>>
>> * A wrong value is used for the GPIO direction.
>>
>> * Wrong offsets are calculated for some GPIO set value/set direction
>>   operations, when offset is larger than 0.
>>
>> This has been fixed by introducing proper manifest constants for the
>> direction encoding, and using 'offsetof' when calculating GPIO
>> register offsets.
>>
>> The updated driver has been tested with the Sparx5 pcb134/pcb135
>> board, which has the mcp2221 device with several (output) GPIO's.
>>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  drivers/hid/hid-mcp2221.c | 48 +++++++++++++++++++++++++++++++--------
>>  1 file changed, 39 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
>> index 0d27ccb55dd9..4211b9839209 100644
>> --- a/drivers/hid/hid-mcp2221.c
>> +++ b/drivers/hid/hid-mcp2221.c
>
>> +/* MCP GPIO get command layout */
>> +struct mcp_get_gpio {
>> +	u8 cmd;
>> +	u8 dummy;
>> +	struct {
>> +		u8 direction;
>> +		u8 value;
>> +	} gpio[MCP_NGPIO];
>> +} __packed;
>
> This bug report: https://bugzilla.kernel.org/show_bug.cgi?id=216736
> suggests that direction and value may be reversed here.
>
> Mentioning here in case nobody actively monitors the bugzilla.
>
> Bjorn
>

I can confirm that the order is reversed when compared to both the data sheet
and what I can observe in the hid reports.
The patch from the linked bugzilla bug fixes the issue for me.

> --- a/drivers/hid/hid-mcp2221.c
> +++ b/drivers/hid/hid-mcp2221.c
> @@ -74,8 +74,8 @@ struct mcp_get_gpio {
>  	u8 cmd;
>  	u8 dummy;
>  	struct {
> -		u8 direction;
>  		u8 value;
> +		u8 direction;
>  	} gpio[MCP_NGPIO];
>  } __packed;

Tested-by: Sven Zühlsdorf <sven.zuehlsdorf@vigem.de>
--8323329-1626605204-1670602151=:61397--