From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 41D8DC49EA7 for ; Thu, 24 Jun 2021 23:11:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 23B3C61351 for ; Thu, 24 Jun 2021 23:11:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232785AbhFXXN4 (ORCPT ); Thu, 24 Jun 2021 19:13:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49974 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229585AbhFXXNt (ORCPT ); Thu, 24 Jun 2021 19:13:49 -0400 Received: from mail-lf1-x134.google.com (mail-lf1-x134.google.com [IPv6:2a00:1450:4864:20::134]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0CFC8C06175F for ; Thu, 24 Jun 2021 16:11:29 -0700 (PDT) Received: by mail-lf1-x134.google.com with SMTP id h15so13026267lfv.12 for ; Thu, 24 Jun 2021 16:11:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RKvQ84L7i7Q66+4hgvpmihpYU0bPeDpVb9EJphxZQuI=; b=JHg/jc0bbwBhVvdYJ5vx8d2GNcCz+NMvF8A3yduFH0LYdaCwsrnchIU0jhqhz5Ve2Z f4dY09j0Xb0kkjv2omyezcrJQQGA077N47WmrXQMHxkvB3IngzZUknQOk/ikkVSbhbWG +FVtuHVD3ajJ+OcnzMKRfaz8agcde6DoG2XMBzAlxHMvs3k9Ag5WEguLTXZSm6uZsO9n nEPASYO+9eXNZOKhcTP95ddDQ3VWfXP04dnJsBnFo+u7tliAgiTkSMBQ+PK4RSodCqKj JCV5EeOxSMU7F3csKlTCCkhzFUQaSKc3WXuLfcJVkzJWy7WNAGYiCgX8AuHQouhP0Owr 5n2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RKvQ84L7i7Q66+4hgvpmihpYU0bPeDpVb9EJphxZQuI=; b=j/6zntnVgl61F/njEnp7ByE72aZoV8Ow4pbwjvQvuSZbpCYdwPrV4p8xlR6fmddmQu /F9Jb5+LfpPuGHxt6NY8UNm1Z36m7nHHJMLDqLNajgR/rMihPmKSN/fIFVzCbY8i2l7L P2uZKejRrUCrGUicpuwJx6cZDYnCGPQhWZ/yZG0gneuvy1BCAVorYLp/4NLR6wOfEKD3 ZIa/n5WQcQPpk5osnHdUgM+kp0eWIoxEjopRZ0Mid36GCb+cMn+vFiDLcVB8alvBbwDa 0ZBOtrJCCNHVUgGlHnGpzuT5LAU5Pu0JCYTVt53cVqvg293L6RwBYgbiazDSy+rg85Mq WAfA== X-Gm-Message-State: AOAM532xal5K/PvvAX2gbqarK5teUglENjE0EyVgTeT9RIYWQxFN3JVR V7vanxMnGASelPWf3Fl7adKquG9gpEmyYhzEDPKiLg== X-Google-Smtp-Source: ABdhPJyom1TBXNm48PvJfgmrVt21Lq9BwYs6rnP7RXGGEJS3orfgEWQ3PsfcdUgopwph82aVwwuXTZhWBgPlhN4YIOg= X-Received: by 2002:ac2:4d25:: with SMTP id h5mr3565495lfk.291.1624576287158; Thu, 24 Jun 2021 16:11:27 -0700 (PDT) MIME-Version: 1.0 References: <20210531090540.2663171-1-luca@lucaceresoli.net> <20210531133211.llyiq3jcfy25tmz4@pali> <8ff1c54f-bb29-1e40-8342-905e34361e1c@lucaceresoli.net> <9fdbada4-4902-cec1-f283-0d12e1d4ac64@ti.com> <20210531162242.jm73yzntzmilsvbg@pali> <8207a53c-4de9-d0e5-295a-c165e7237e36@lucaceresoli.net> <20210622110627.aqzxxtf2j3uxfeyl@pali> <20210622115604.GA25503@lpieralisi> <20210622121649.ouiaecdvwutgdyy5@pali> <18a104a9-2cb8-7535-a5b2-f5f049adff47@lucaceresoli.net> <4d4c0d4d-41b4-4756-5189-bffa15f88406@ti.com> In-Reply-To: <4d4c0d4d-41b4-4756-5189-bffa15f88406@ti.com> From: Linus Walleij Date: Fri, 25 Jun 2021 01:11:15 +0200 Message-ID: Subject: Re: [PATCH v2] PCI: dra7xx: Fix reset behaviour To: Kishon Vijay Abraham I , Bartosz Golaszewski Cc: Luca Ceresoli , =?UTF-8?Q?Pali_Roh=C3=A1r?= , Lorenzo Pieralisi , linux-pci , Linux-OMAP , Linux ARM , linux-kernel , Rob Herring , Bjorn Helgaas Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 22, 2021 at 3:57 PM Kishon Vijay Abraham I wrote: > While the patch itself is correct, this kind-of changes the behavior on > already upstreamed platforms. Previously the driver expected #PERST to > be asserted be external means (or default power-up state) and only takes > care of de-asserting the #PERST line. > > There are 2 platforms that will be impacted due to this change > 1) arch/arm/boot/dts/am57xx-beagle-x15-common.dist (has an inverter on > GPIO line) > 2) arch/arm/boot/dts/am571x-idk.dts (directly connected to #PERST) > > For 1), gpiod_set_value(reset, 0) will assert the PERST line due to the > inverter (and GPIO_ACTIVE_LOW) > For 2), gpiod_set_value(reset, 0) will assert the PERST line because we > have GPIO_ACTIVE_HIGH The presence of an inverter makes it necessary to model this the right way to get out of the situation. > So this patch should have to be accompanied with DT changes (and this > patch also breaks old DT compatibility). There are ways to deal with this perfectly. It may or may not be worth the extra work. But I can show how it is done. Make the patch to the driver that assumes driving the gpio descriptor to 1 (asserted) has the desired effect. In this patch, I would include a hunk that fixes the above device trees, so they are correct from this point. This is one of the few cases where I think it warrants to fix the driver and the DTS file at the same time, but the DTS can also be patched separately because of the described solution below: To avoid regressions with old device trees, add code to drivers/gpio/gpiolib-of.c in function of_gpio_flags_quirks() to react to the old incorrect device trees. This is where we stockpile OF errors and bug fixes. This needs to be pretty elaborate. It begins like this: if (IS_ENABLED(CONFIG_PCI) && (of_machine_is_compatible("ti,am572x-beagle-x15") || of_machine_is_compatible("ti,am5718-idk")) && of_node_name_eq(np, "pcie")) { /* ... add code to check and enforce the flags ... */ } You see the idea here. Include this in the patch to make the Perfect(TM) solution to this problem both fixing all device trees in place and dealing with the old erroneous ones using some elaborate code. There are plenty of examples on how to detect warn and modify flags in of_gpio_flags_quirks() make it clear and add some warning prints and comments. Keep me and Bartosz in the loop. It'll look fine in the end. Yours, Linus Walleij From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E679C49EA5 for ; Thu, 24 Jun 2021 23:13:05 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 158E761351 for ; Thu, 24 Jun 2021 23:13:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 158E761351 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TQOZK2N1O7ooRlFJHPy16o0pLI93ThC9+T8rcdrsRMI=; b=W2z2LXSoM0kZXU MLZN3a3h64uM6D1PokHioDk4frXljMuE8/t2o85Uo9xMJRI2qoZD7Ul8gCElUb714NjVKnHP3d9X7 DFreL/ZhrotBviCS0ysMX7C7nwtbdTWUIinYZRTxEFusg11cgoHBogKg6dFYt1QbIhDzs3SVtBIb8 0kNeZzumeTRKT/bmFsyda/wynt5+h/HHrPsNFwLaFujZlhpmQX5+5ZEzySEl9SonbzR3xN2yIP+CT w4scmFci6scVB0FdhdNE06UZ/87LuFm75UhngXN+clbWE3Pm+oZrZpMed8EyFgOCuPTn2hXm4ha75 Xpxs+V6wxyIhmJWGi9ZA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lwYVQ-00Ghk1-PV; Thu, 24 Jun 2021 23:11:32 +0000 Received: from mail-lf1-x135.google.com ([2a00:1450:4864:20::135]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lwYVN-00Ghja-9P for linux-arm-kernel@lists.infradead.org; Thu, 24 Jun 2021 23:11:30 +0000 Received: by mail-lf1-x135.google.com with SMTP id a11so13059308lfg.11 for ; Thu, 24 Jun 2021 16:11:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RKvQ84L7i7Q66+4hgvpmihpYU0bPeDpVb9EJphxZQuI=; b=JHg/jc0bbwBhVvdYJ5vx8d2GNcCz+NMvF8A3yduFH0LYdaCwsrnchIU0jhqhz5Ve2Z f4dY09j0Xb0kkjv2omyezcrJQQGA077N47WmrXQMHxkvB3IngzZUknQOk/ikkVSbhbWG +FVtuHVD3ajJ+OcnzMKRfaz8agcde6DoG2XMBzAlxHMvs3k9Ag5WEguLTXZSm6uZsO9n nEPASYO+9eXNZOKhcTP95ddDQ3VWfXP04dnJsBnFo+u7tliAgiTkSMBQ+PK4RSodCqKj JCV5EeOxSMU7F3csKlTCCkhzFUQaSKc3WXuLfcJVkzJWy7WNAGYiCgX8AuHQouhP0Owr 5n2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RKvQ84L7i7Q66+4hgvpmihpYU0bPeDpVb9EJphxZQuI=; b=pMAbFkvxxCsKrHXW+kG2S8NbjY538ZWSr9nCR1JfluKwm2FFXEEpQwOJp1LXY0dfJ3 zwHxZI3SzaxOpD3QHf31KyEjf5KkEYBR326Un7Ndgt+xb1JOCN7lK1sdmIcVz4H5tNCQ SFwrOjRYVV83RuD79Tn/T+FTsVPKZg14JZ+39IjhvJ7FQJXsUFkpp5zeJJsyezVya35A uPWIkxkPr5RhPxO2r9cxBxALIAcmgDyaK5GruLiiWqvu1RIRust7QZdOmJKmrguMlq5s br+/2G5tBiKUuCm/z2YZUFqKZksfVuh3sCPP25l1jioHDrDYzvjkKtCpYlvh7BUvx+Ni fYKw== X-Gm-Message-State: AOAM532/q2MLcvy/74o7NYVn9uZ/iDOVdY8fHXWk7GZ2+Tvn2d2sGEHx Q4UaImaSIs7UmoES2E29Mus83xsKtv8DXu1Tg7bR/w== X-Google-Smtp-Source: ABdhPJyom1TBXNm48PvJfgmrVt21Lq9BwYs6rnP7RXGGEJS3orfgEWQ3PsfcdUgopwph82aVwwuXTZhWBgPlhN4YIOg= X-Received: by 2002:ac2:4d25:: with SMTP id h5mr3565495lfk.291.1624576287158; Thu, 24 Jun 2021 16:11:27 -0700 (PDT) MIME-Version: 1.0 References: <20210531090540.2663171-1-luca@lucaceresoli.net> <20210531133211.llyiq3jcfy25tmz4@pali> <8ff1c54f-bb29-1e40-8342-905e34361e1c@lucaceresoli.net> <9fdbada4-4902-cec1-f283-0d12e1d4ac64@ti.com> <20210531162242.jm73yzntzmilsvbg@pali> <8207a53c-4de9-d0e5-295a-c165e7237e36@lucaceresoli.net> <20210622110627.aqzxxtf2j3uxfeyl@pali> <20210622115604.GA25503@lpieralisi> <20210622121649.ouiaecdvwutgdyy5@pali> <18a104a9-2cb8-7535-a5b2-f5f049adff47@lucaceresoli.net> <4d4c0d4d-41b4-4756-5189-bffa15f88406@ti.com> In-Reply-To: <4d4c0d4d-41b4-4756-5189-bffa15f88406@ti.com> From: Linus Walleij Date: Fri, 25 Jun 2021 01:11:15 +0200 Message-ID: Subject: Re: [PATCH v2] PCI: dra7xx: Fix reset behaviour To: Kishon Vijay Abraham I , Bartosz Golaszewski Cc: Luca Ceresoli , =?UTF-8?Q?Pali_Roh=C3=A1r?= , Lorenzo Pieralisi , linux-pci , Linux-OMAP , Linux ARM , linux-kernel , Rob Herring , Bjorn Helgaas X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210624_161129_382090_E603BEF7 X-CRM114-Status: GOOD ( 24.08 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jun 22, 2021 at 3:57 PM Kishon Vijay Abraham I wrote: > While the patch itself is correct, this kind-of changes the behavior on > already upstreamed platforms. Previously the driver expected #PERST to > be asserted be external means (or default power-up state) and only takes > care of de-asserting the #PERST line. > > There are 2 platforms that will be impacted due to this change > 1) arch/arm/boot/dts/am57xx-beagle-x15-common.dist (has an inverter on > GPIO line) > 2) arch/arm/boot/dts/am571x-idk.dts (directly connected to #PERST) > > For 1), gpiod_set_value(reset, 0) will assert the PERST line due to the > inverter (and GPIO_ACTIVE_LOW) > For 2), gpiod_set_value(reset, 0) will assert the PERST line because we > have GPIO_ACTIVE_HIGH The presence of an inverter makes it necessary to model this the right way to get out of the situation. > So this patch should have to be accompanied with DT changes (and this > patch also breaks old DT compatibility). There are ways to deal with this perfectly. It may or may not be worth the extra work. But I can show how it is done. Make the patch to the driver that assumes driving the gpio descriptor to 1 (asserted) has the desired effect. In this patch, I would include a hunk that fixes the above device trees, so they are correct from this point. This is one of the few cases where I think it warrants to fix the driver and the DTS file at the same time, but the DTS can also be patched separately because of the described solution below: To avoid regressions with old device trees, add code to drivers/gpio/gpiolib-of.c in function of_gpio_flags_quirks() to react to the old incorrect device trees. This is where we stockpile OF errors and bug fixes. This needs to be pretty elaborate. It begins like this: if (IS_ENABLED(CONFIG_PCI) && (of_machine_is_compatible("ti,am572x-beagle-x15") || of_machine_is_compatible("ti,am5718-idk")) && of_node_name_eq(np, "pcie")) { /* ... add code to check and enforce the flags ... */ } You see the idea here. Include this in the patch to make the Perfect(TM) solution to this problem both fixing all device trees in place and dealing with the old erroneous ones using some elaborate code. There are plenty of examples on how to detect warn and modify flags in of_gpio_flags_quirks() make it clear and add some warning prints and comments. Keep me and Bartosz in the loop. It'll look fine in the end. Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel