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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 95917C433FE for ; Wed, 30 Nov 2022 12:33:31 +0000 (UTC) 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:In-Reply-To:MIME-Version:References: 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=dXi1r6V9QzwfU4bsFq5oiz+iyLjK94b1wrR7897YCOw=; b=B7Zp5E3IH3tQor YzOReffNcZb7iaB2PadS0BF9pMWgJ9LvJBAyco1d4DIfD6babI+BsxV+CpxRa5rfPBknnLu+zIFEL kB6kd4wo2h8DxTTODV3fiyGolihjbNNSeox2P7U0qfzS2HC4OzKnR1hj2bRsoTib6tJWghZKbcDbk njhAS5wvJunDYlkf0pb/BTHDM2tQ3JvuELEO65HkNcpmERv87sykhyzHUpCye+Dkp+dkRjhOTlMxf EMSy59P1yxvH1y+157VFjvx9Y+iCGy0yE/RfN1COEgBByDbUQI+VhCcbecFrZ/+94VqHRNG4ztqke TQNKiu5/F9M2snS8415A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0MGP-00G8db-3U; Wed, 30 Nov 2022 12:32:33 +0000 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p0MGK-00G8au-Ar for linux-arm-kernel@lists.infradead.org; Wed, 30 Nov 2022 12:32:30 +0000 Received: by mail-ej1-x632.google.com with SMTP id ha10so41029455ejb.3 for ; Wed, 30 Nov 2022 04:32:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=csbn9ZA9ZuwCO7vca9fwDmQPDYbQoHiHPP6TTlnfzgY=; b=Kb4vLWPGEHi8yeDUacFHkNuvQDyQusoagzzW9VEjPTFiqT5OTDpEtEJYZFw7V830jj lPEM4sEuOzqtm7UzC0agwCcatSTBOIAq3xKpM2Rl+x83tudbGL9D0RpDDNj0mQplBjQE 348G6xfvsx+M5zq8UZBHsyd1Ysst+uloMk6A+CqukUvdipTYypG2al0yRLeVneL3YSgF bGBYeTY0G3SzmrZvQ/GZfMx4P+Se52rFnuLTm1RZASdhJMieLUGa48OnpMVUuX7Fg3yo YhP3hiPFwKqyBzYWhF69l60llDb0jkd52geAhN4VMcGGYpu6+eqa0lBOM4it8rnHoZmi iMHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=csbn9ZA9ZuwCO7vca9fwDmQPDYbQoHiHPP6TTlnfzgY=; b=powHYuNtbKDfLK4kA5rmCWkF1CT+mz3F08x6vc58NAGJMPHfrBYr0Tqo4W3VoEGibo 4I9FK2Iho63rU26JS8yHaaPPq+IoNrcObGKptvgpJaNzksb97FGbBA4i8TB9/dShh4HR PA83HEgihPADxeevGhACpjqhrRPavg+D2QT/mDpZE6Zd9hugWigXRArqutvOW0eQ3uHp +NIp2u3Ds7kgwMMcLwo8S4vBzNMWbKaHjNlQpnda+KhmPVrvKHdPLuwg/iF8AN4yXsNS 5VwhD3K1jA23KBHwuJ/S2qPCGdV0bXrLtwACNT+I1poYNspTvS4+hQVkzh1+7VJd915Q oUgA== X-Gm-Message-State: ANoB5plpcHg1lFNB3ZEhRW1rXrD/xhTwECpETh5sTvDV00wpLis3PglU EPK2GtXrxwILgYkewaPL5RXrJw== X-Google-Smtp-Source: AA0mqf73IYitcwwy+EOHIAFmV/8X7Uv20xNLcSHFr78y8bO/rSsdAwSc/FVBomlP6JP6xZbqOpEIew== X-Received: by 2002:a17:906:a387:b0:78d:946e:f65d with SMTP id k7-20020a170906a38700b0078d946ef65dmr52098133ejz.365.1669811544914; Wed, 30 Nov 2022 04:32:24 -0800 (PST) Received: from localhost ([86.61.181.4]) by smtp.gmail.com with ESMTPSA id s17-20020a05640217d100b004585eba4baesm568982edy.80.2022.11.30.04.32.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Nov 2022 04:32:24 -0800 (PST) Date: Wed, 30 Nov 2022 13:32:23 +0100 From: Jiri Pirko To: Vadim Fedorenko Cc: Jakub Kicinski , Arkadiusz Kubalewski , Jonathan Lemon , Paolo Abeni , netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API Message-ID: References: <20221129213724.10119-1-vfedorenko@novek.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221129213724.10119-1-vfedorenko@novek.ru> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221130_043228_399943_38ACB00E X-CRM114-Status: GOOD ( 20.68 ) 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 Tue, Nov 29, 2022 at 10:37:20PM CET, vfedorenko@novek.ru wrote: >Implement common API for clock/DPLL configuration and status reporting. >The API utilises netlink interface as transport for commands and event >notifications. This API aim to extend current pin configuration and >make it flexible and easy to cover special configurations. Overall, I see a lot of issues on multiple levels. I will go over them in follow-up emails. So far, after couple of hours looking trought this, I have following general feelings/notes: 1) Netlink interface looks much saner than in previous versions. I will send couple of notes, mainly around events and object mixtures and couple of bugs/redundancies. But overall, looks fineish. 2) I don't like that concept of a shared pin, at all. It makes things unnecessary complicated. Just have a pin created for dpll instance and that's it. If another instance has the same pin, it should create it as well. Keeps things separate and easy to model. Let the hw/fw/driver figure out the implementation oddities. Why exactly you keep pushing the shared pin idea? Perhaps I'm missing something crucial. 3) I don't like the concept of muxed pins and hierarchies of pins. Why does user care? If pin is muxed, the rest of the pins related to this one should be in state disabled/disconnected. The user only cares about to see which pins are related to each other. It can be easily exposed by "muxid" like this: pin 1 pin 2 pin 3 muxid 100 pin 4 muxid 100 pin 5 muxid 101 pin 6 muxid 101 In this example pins 3,4 and 5,6 are muxed, therefore the user knows if he connects one, the other one gets disconnected (or will have to disconnect the first one explicitly first). 4) I don't like the "attr" indirection. It makes things very tangled. It comes from the concepts of classes and objects and takes it to extreme. Not really something we are commonly used to in kernel. Also, it brings no value from what I can see, only makes things very hard to read and follow. Please keep things direct and simple: * If some option could be changed for a pin or dpll, just have an op that is directly called from netlink handler to change it. There should be clear set of ops for configuration of pin and dpll object. This "attr" indirection make this totally invisible. * If some attribute is const during dpll or pin lifetime, have it passed during dpll or pin creation. > >v3 -> v4: > * redesign framework to make pins dynamically allocated (Arkadiusz) > * implement shared pins (Arkadiusz) >v2 -> v3: > * implement source select mode (Arkadiusz) > * add documentation > * implementation improvements (Jakub) >v1 -> v2: > * implement returning supported input/output types > * ptp_ocp: follow suggestions from Jonathan > * add linux-clk mailing list >v0 -> v1: > * fix code style and errors > * add linux-arm mailing list > > >Arkadiusz Kubalewski (1): > dpll: add dpll_attr/dpll_pin_attr helper classes > >Vadim Fedorenko (3): > dpll: Add DPLL framework base functions > dpll: documentation on DPLL subsystem interface > ptp_ocp: implement DPLL ops > > Documentation/networking/dpll.rst | 271 ++++++++ > Documentation/networking/index.rst | 1 + > MAINTAINERS | 8 + > drivers/Kconfig | 2 + > drivers/Makefile | 1 + > drivers/dpll/Kconfig | 7 + > drivers/dpll/Makefile | 11 + > drivers/dpll/dpll_attr.c | 278 +++++++++ > drivers/dpll/dpll_core.c | 760 +++++++++++++++++++++++ > drivers/dpll/dpll_core.h | 176 ++++++ > drivers/dpll/dpll_netlink.c | 963 +++++++++++++++++++++++++++++ > drivers/dpll/dpll_netlink.h | 24 + > drivers/dpll/dpll_pin_attr.c | 456 ++++++++++++++ > drivers/ptp/Kconfig | 1 + > drivers/ptp/ptp_ocp.c | 123 ++-- > include/linux/dpll.h | 261 ++++++++ > include/linux/dpll_attr.h | 433 +++++++++++++ > include/uapi/linux/dpll.h | 263 ++++++++ > 18 files changed, 4002 insertions(+), 37 deletions(-) > create mode 100644 Documentation/networking/dpll.rst > create mode 100644 drivers/dpll/Kconfig > create mode 100644 drivers/dpll/Makefile > create mode 100644 drivers/dpll/dpll_attr.c > create mode 100644 drivers/dpll/dpll_core.c > create mode 100644 drivers/dpll/dpll_core.h > create mode 100644 drivers/dpll/dpll_netlink.c > create mode 100644 drivers/dpll/dpll_netlink.h > create mode 100644 drivers/dpll/dpll_pin_attr.c > create mode 100644 include/linux/dpll.h > create mode 100644 include/linux/dpll_attr.h > create mode 100644 include/uapi/linux/dpll.h > >-- >2.27.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel