From patchwork Mon Aug 13 21:16:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Lengfeld X-Patchwork-Id: 957274 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-i2c-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=stefanchrist.eu Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41q7nn0hyJz9s7c for ; Tue, 14 Aug 2018 07:16:28 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729126AbeHNAAT (ORCPT ); Mon, 13 Aug 2018 20:00:19 -0400 Received: from stcim.de ([78.46.90.227]:34458 "EHLO stcim.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730050AbeHNAAT (ORCPT ); Mon, 13 Aug 2018 20:00:19 -0400 Received: from xdsl-87-78-37-34.netcologne.de ([87.78.37.34] helo=localhost.localdomain) by stcim with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.89) (envelope-from ) id 1fpKCF-0008Ap-9u; Mon, 13 Aug 2018 23:16:15 +0200 From: Stefan Lengfeld To: linux-i2c@vger.kernel.org Cc: wsa@the-dreams.de, preid@electromag.com.au, j-keerthy@ti.com, tony@atomide.com, nsekhar@ti.com, vigneshr@ti.com, linux-omap@vger.kernel.org Subject: [RFC PATCH 0/4] I2C writes with interrupts disabled Date: Mon, 13 Aug 2018 23:16:10 +0200 Message-Id: <20180813211614.16440-1-contact@stefanchrist.eu> X-Mailer: git-send-email 2.16.4 In-Reply-To: <5bb4b898-acc3-7c73-2285-cf84a88961e6@ti.com> References: <5bb4b898-acc3-7c73-2285-cf84a88961e6@ti.com> Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Hi all, to continue the discussion: Here is a new spin of an I2C IRQ less patch set. The approach does not introduce a new I2C transfer callback like master_xfer_irqless(). Instead if follows Tero Kristo first patch and the I2C driver code should detect itself, whether it's called in IRQ disabled context with code like bool polling = irqs_disabled(); Using this function seems appropriate, because the I2C core does the same in the function i2c_transfer(). Furthermore the patchset introduces a boolean flag 'irq_safe' in the i2c_adapter. Every driver author should announce in the driver's probe function whether the driver is safe to be called in atomic or IRQ disabled contexts with the function i2c_adapter_irq_safe(). The idea is taken from the PM subsystem and the function pm_runtime_irq_safe(). The i2c_adapter_irq_safe() function should address Wolfram Sang's comment: > * Also, there is no white-listing, all transfers will be executed, so > buggy drivers will go unnoticed. Drivers that are not coded with IRQ less operation in mind get noticed at once. Responding to Tero Kristo's comment: > So, what is the plan going forward? > > 1) Modify i2c core APIs? And modify all relevant i2c drivers to support this? > 2) Just add the shutdown handler switch to the relevant drivers? (My RFCv2 > patch) > 3) Something else? This patchset combines your RFCv1 patch with a minimal I2C core change. For Wolfram Sang's comment: > IIRC the latest design we discussed is to add a new callback to struct > i2c_algorithm like 'master_xfer_irqless' and teach the I2C core when to > call which callback. Which might not be so super straightforward because > for most drivers (except PMICs probably) using I2C when interrupts are > disabled is a bug and we also shouldn't hide that by providing a generic > fallback. To detect buggy users of the I2C API, we would need to change the I2C API and add a function like: i2c_transfer(); // existing function i2c_transfer_irqless(); // new function for PMIC drivers Using a flag like 'I2C_M_IRQLESS' seems wrong, because IRQ less operation is a per-transfer, not per message thing. Nevertheless, as you already said, such a new API function would required tree-wide changes, e.g in the regmap framework. For my comment: > There are two distinct problems: > * Sending I2C messages in an atomic/irqless/sleep-free context > * Blocking the I2C-adapter for other users before starting the > sequence poweroff/reboot. The second issue I cannot reproduce anymore. Maybe it's due to having a single core i.MX6 or something else. So the corresponding patch to modify the I2C API is left out in the patchset. I Just a not for development the code. Without the help of the LOCKDEP framework and config options: CONFIG_PROVE_LOCKING=y CONFIG_DEBUG_ATOMIC_SLEEP=y it's nearly impossible two write sleep/schedule free code. There are just to many kernel functions that sleep internally. The patches are tested on a kernel v4.18-rc8 and a phyCORE-i.MX6 Solo board. Kind regards, Stefan Lengfeld Stefan Christ (1): ARM: dts: phyboard-mira-dl: rely on PMIC for reboot and watchdog Stefan Lengfeld (3): i2c: allow drivers to announce that they are IRQ safe i2c: imx: implement IRQ less master_xfer function watchdog: da9062: avoid regmap in restart handler arch/arm/boot/dts/imx6dl-phytec-mira-rdk-nand.dts | 8 ++ drivers/i2c/busses/i2c-imx.c | 113 ++++++++++++++++------ drivers/i2c/i2c-core-base.c | 8 ++ drivers/mfd/da9062-core.c | 1 + drivers/watchdog/da9062_wdt.c | 17 +++- include/linux/i2c.h | 16 +++ include/linux/mfd/da9062/core.h | 1 + 7 files changed, 128 insertions(+), 36 deletions(-)