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=-15.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=unavailable 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 6232FC48BE6 for ; Wed, 16 Jun 2021 10:15:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3F6BF6128B for ; Wed, 16 Jun 2021 10:15:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232171AbhFPKRW (ORCPT ); Wed, 16 Jun 2021 06:17:22 -0400 Received: from foss.arm.com ([217.140.110.172]:32874 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231452AbhFPKRQ (ORCPT ); Wed, 16 Jun 2021 06:17:16 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 334CD1042; Wed, 16 Jun 2021 03:15:10 -0700 (PDT) Received: from slackpad.fritz.box (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5423F3F70D; Wed, 16 Jun 2021 03:15:08 -0700 (PDT) Date: Wed, 16 Jun 2021 11:14:52 +0100 From: Andre Przywara To: Maxime Ripard Cc: Chen-Yu Tsai , Jernej Skrabec , Rob Herring , Icenowy Zheng , Samuel Holland , linux-arm-kernel@lists.infradead.org, linux-sunxi@googlegroups.com, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Ondrej Jirman , Alessandro Zummo , Alexandre Belloni , linux-rtc@vger.kernel.org Subject: Re: [PATCH v7 06/19] rtc: sun6i: Add support for RTCs without external LOSCs Message-ID: <20210616111452.1d7f2423@slackpad.fritz.box> In-Reply-To: <20210616091431.6tm3zdf77p2x3upc@gilmour> References: <20210615110636.23403-1-andre.przywara@arm.com> <20210615110636.23403-7-andre.przywara@arm.com> <20210616091431.6tm3zdf77p2x3upc@gilmour> Organization: Arm Ltd. X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 16 Jun 2021 11:14:31 +0200 Maxime Ripard wrote: Hi, > On Tue, Jun 15, 2021 at 12:06:23PM +0100, Andre Przywara wrote: > > Some newer Allwinner RTCs (for instance the one in the H616 SoC) lack > > a pin for an external 32768 Hz oscillator. As a consequence, this LOSC > > can't be selected as the RTC clock source, and we must rely on the > > internal RC oscillator. > > To allow additions of clocks to the RTC node, add a feature bit to ignore > > any provided clocks for now (the current code would think this is the > > external LOSC). Later DTs and code can then for instance add the PLL > > based clock input, and older kernel won't get confused. > > > > Signed-off-by: Andre Przywara > > Honestly, I don't really know if it's worth it at this point. > > If we sums this up: > > - The RTC has 2 features that we use, mostly centered around 2 > registers set plus a global one > > - Those 2 features are programmed in a completely different way > > - Even the common part is different, given the discussion around the > clocks that we have. > > What is there to share in that driver aside from the probe, and maybe > the interrupt handling? Instead of complicating this further with more > special case that you were (rightfully) complaining about, shouldn't we > just acknowledge the fact that it's a completely separate design and > should be treated as such, with a completely separate driver? If you mean to have a separate clock driver, and one RTC driver: I agree, and IIUC Samuel has a prototype, covering the H6 and D1 as well: https://github.com/smaeul/linux/commit/6f8f761db1d8dd4b6abf006fb7e2427da79321c2 The only problem I see that they are sharing MMIO registers. Maybe it works because the RTC part never touches anything below 0x10, and the clock part just needs the first four registers? But this means we can't easily change this for the H6, as the existing H6 RTC code adds 0x10 to the MMIO base, and also old DTs will have the RTC base address in their RTC reg property. If we can somehow solve this (let the clock driver point to the RTC node to get a regmap?) I am all in, for the reasons you mentioned. Cheers, Andre 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=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=unavailable 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 37D5CC48BE5 for ; Wed, 16 Jun 2021 10:16:42 +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 0496361153 for ; Wed, 16 Jun 2021 10:16:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0496361153 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com 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:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=xiMU/5R4VA8hMuxHcj4Z0ffCupPK6ZZqGHKS/rPuvpU=; b=Se0Q0UlPUuBePh zoidM0vuLRwP5FFN9/aQiyl0BZRgvcwgPGXaOZtMqQo54lbofPiPZjsAaOJ4eYvY8SW9mej0IHQcX pMdmpWv1jNCWAheCCIIopPFwY7BEWprP2RHusPab94Yyot1AXOujlnzWa/2mWEgIhrJSC1xz6dHsp 4rctVCbKlvBNhKuhnl2zKhzkYR/lK0PxQmlJ5neXHcurUBJ4t2GCZw4M7t95MN8cjad45nQJqCzLb 8qC27MpKeAu5EXkgf9gCrD9wb5xaQNimydVIdpYGWW/UlnHp9yMz20eu9AO0htHXF9V9hfR4K8mkx xH6yqiaTRhA1N2ifXxnA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ltSZp-005jU7-Fn; Wed, 16 Jun 2021 10:15:17 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ltSZl-005jTM-3t for linux-arm-kernel@lists.infradead.org; Wed, 16 Jun 2021 10:15:14 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 334CD1042; Wed, 16 Jun 2021 03:15:10 -0700 (PDT) Received: from slackpad.fritz.box (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5423F3F70D; Wed, 16 Jun 2021 03:15:08 -0700 (PDT) Date: Wed, 16 Jun 2021 11:14:52 +0100 From: Andre Przywara To: Maxime Ripard Cc: Chen-Yu Tsai , Jernej Skrabec , Rob Herring , Icenowy Zheng , Samuel Holland , linux-arm-kernel@lists.infradead.org, linux-sunxi@googlegroups.com, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Ondrej Jirman , Alessandro Zummo , Alexandre Belloni , linux-rtc@vger.kernel.org Subject: Re: [PATCH v7 06/19] rtc: sun6i: Add support for RTCs without external LOSCs Message-ID: <20210616111452.1d7f2423@slackpad.fritz.box> In-Reply-To: <20210616091431.6tm3zdf77p2x3upc@gilmour> References: <20210615110636.23403-1-andre.przywara@arm.com> <20210615110636.23403-7-andre.przywara@arm.com> <20210616091431.6tm3zdf77p2x3upc@gilmour> Organization: Arm Ltd. X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210616_031513_280617_22BE5CDB X-CRM114-Status: GOOD ( 28.32 ) 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 Wed, 16 Jun 2021 11:14:31 +0200 Maxime Ripard wrote: Hi, > On Tue, Jun 15, 2021 at 12:06:23PM +0100, Andre Przywara wrote: > > Some newer Allwinner RTCs (for instance the one in the H616 SoC) lack > > a pin for an external 32768 Hz oscillator. As a consequence, this LOSC > > can't be selected as the RTC clock source, and we must rely on the > > internal RC oscillator. > > To allow additions of clocks to the RTC node, add a feature bit to ignore > > any provided clocks for now (the current code would think this is the > > external LOSC). Later DTs and code can then for instance add the PLL > > based clock input, and older kernel won't get confused. > > > > Signed-off-by: Andre Przywara > > Honestly, I don't really know if it's worth it at this point. > > If we sums this up: > > - The RTC has 2 features that we use, mostly centered around 2 > registers set plus a global one > > - Those 2 features are programmed in a completely different way > > - Even the common part is different, given the discussion around the > clocks that we have. > > What is there to share in that driver aside from the probe, and maybe > the interrupt handling? Instead of complicating this further with more > special case that you were (rightfully) complaining about, shouldn't we > just acknowledge the fact that it's a completely separate design and > should be treated as such, with a completely separate driver? If you mean to have a separate clock driver, and one RTC driver: I agree, and IIUC Samuel has a prototype, covering the H6 and D1 as well: https://github.com/smaeul/linux/commit/6f8f761db1d8dd4b6abf006fb7e2427da79321c2 The only problem I see that they are sharing MMIO registers. Maybe it works because the RTC part never touches anything below 0x10, and the clock part just needs the first four registers? But this means we can't easily change this for the H6, as the existing H6 RTC code adds 0x10 to the MMIO base, and also old DTs will have the RTC base address in their RTC reg property. If we can somehow solve this (let the clock driver point to the RTC node to get a regmap?) I am all in, for the reasons you mentioned. Cheers, Andre _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel