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 CFAEAC76188 for ; Wed, 5 Apr 2023 14:31:53 +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:MIME-Version:Message-ID:In-reply-to: Date:Subject:Cc:To:From:References:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=GgI9z9qxUsQ/9he9V/QgO+iw/YQ/3YPBolFIHUrZQkM=; b=Bet4NvEm24CkYd HZqCSok4BELArx9leRL+kehGX6laNNT+6C2/SUwWxTaNYjanPDiFbeI0WGqLHY8FF3M1xZcnuX6VK Z+vDxWxs08gBmODxSXLDlVEwwNX0bnDv6h0YgVGbb2hGfIMDkt1t89eY9I7r4XozUMWfjhRUf6Jv0 VarpFqZqLTCoruto6WukkAV1PsSCCWrVjmA2ESEeyltc92dEU+tz8kJVtH3rbbUhVW0BeIEwXYxrw vTcYzfmuTelytv47QKPeLQ9A6dyvXg/2GVfX+udG2CMsi/GPS3D9XfYvYnoRPOK/gG4kHZcajomDp R5KZ847U2SJbSwTEwC1Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pk4A7-004gVc-1b; Wed, 05 Apr 2023 14:30:59 +0000 Received: from mail-wr1-x434.google.com ([2a00:1450:4864:20::434]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pk4A3-004gTw-1w for linux-arm-kernel@lists.infradead.org; Wed, 05 Apr 2023 14:30:58 +0000 Received: by mail-wr1-x434.google.com with SMTP id m2so36421845wrh.6 for ; Wed, 05 Apr 2023 07:30:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; t=1680705052; h=mime-version:message-id:in-reply-to:date:subject:cc:to:from :user-agent:references:from:to:cc:subject:date:message-id:reply-to; bh=EgLadd94O8XczIdn8YeDNTRC0ay46l/3/7AZ410rDto=; b=FWJq9Q1aODrqJntWSOyXbnNZ0SBHSDnck8NBF5YwVSAa5xsAoKATvHscFzmJaT+ssV +qHyfK7v+pgz3Sw4j4sSkWTYWXdSY40bEr5dYJjOrknxaIiWcDC4ZbdCQKgHsbotcHYK ue8qZ+qKHWP78RU0P7X/t03TR9uQG9XRUKr479v3Rjmy7Q1gOOygYnBZiAWtWAilvVaJ /Td0/17FzBRb6LHFhbcdu7ZeTEv1f4fyTRrw7oBRi6SyQ5hZj40m4KY/y/9FptlV8sUD s5ryAfBonG1trHXmoPOD8JIa3ZUxIiX1WDrQpX/giovXjA0gK26bPFhjlEYmROb732gy 3WDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680705052; h=mime-version:message-id:in-reply-to:date:subject:cc:to:from :user-agent:references:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=EgLadd94O8XczIdn8YeDNTRC0ay46l/3/7AZ410rDto=; b=FRtCPgGxtW1x7pnGql+X3uBysxfuQKicaPIYO0Vpt9l7GGP6pk6kHTm0RHDCCPR1gs ah7GHNcKolGSzf3N4wWJY/vHzByDHcXlOubH1MMgk06yqSGc6QgYR+dK9PcLP+relB+P pwSuPFcWIkaxF8SQXjQHd5P7pa2LybG+0p52Zi9h0x0s9AUrIbOybkg+1YTs8BPDKdKZ SxFhQKf9PvtyEb8HE9BdbePXQ/BcqN1C7qaY+u21KEXzTLpDRh9DBYrkJWtCVFechgU+ 9lW/RIrNO5NgzppekfP2JI5QFpDLVDjynSMdYiB15KJNEL8fhdCE5rJ38IM2Ffl3O4fC L53Q== X-Gm-Message-State: AAQBX9cQtLqO+WxobLoZdx5l/+6hvs5NH6LSH0OY0MABX8wStNZ85zVB MqHq60AoqLGF0bosVSavna463g== X-Google-Smtp-Source: AKy350bKIW36MwRanv80akKe3S3w+vDG8PDIWtrl9tvFe8SCq2HgcGIvbVqSLvzGKUvHr5qZAdkeIw== X-Received: by 2002:a5d:6391:0:b0:2e4:cc1c:1036 with SMTP id p17-20020a5d6391000000b002e4cc1c1036mr4406813wru.62.1680705051753; Wed, 05 Apr 2023 07:30:51 -0700 (PDT) Received: from localhost (laubervilliers-658-1-213-31.w90-63.abo.wanadoo.fr. [90.63.244.31]) by smtp.gmail.com with ESMTPSA id r8-20020a5d4e48000000b002c70e60abd4sm15370114wrt.2.2023.04.05.07.30.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Apr 2023 07:30:51 -0700 (PDT) References: <20230404155332.9571-1-ddrokosov@sberdevices.ru> <20230404155332.9571-7-ddrokosov@sberdevices.ru> User-agent: mu4e 1.8.13; emacs 28.2 From: Jerome Brunet To: Dmitry Rokosov , neil.armstrong@linaro.org, mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, khilman@baylibre.com, martin.blumenstingl@googlemail.com Cc: jian.hu@amlogic.com, kernel@sberdevices.ru, rockosov@gmail.com, linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v12 6/6] clk: meson: a1: add Amlogic A1 Peripherals clock controller driver Date: Wed, 05 Apr 2023 16:13:44 +0200 In-reply-to: <20230404155332.9571-7-ddrokosov@sberdevices.ru> Message-ID: <1j8rf6flk5.fsf@starbuckisacylon.baylibre.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230405_073055_811899_7C562056 X-CRM114-Status: GOOD ( 12.50 ) 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 04 Apr 2023 at 18:53, Dmitry Rokosov wrote: > +/* Array of all clocks registered by this provider */ > +static struct clk_hw_onecell_data a1_periphs_clks = { > + .hws = { > + /* DT exposed clocks */ > + [CLKID_FIXPLL_IN] = &fixpll_in.hw, > + [CLKID_USB_PHY_IN] = &usb_phy_in.hw, > + [CLKID_USB_CTRL_IN] = &usb_ctrl_in.hw, > + [CLKID_HIFIPLL_IN] = &hifipll_in.hw, > + [CLKID_SYSPLL_IN] = &syspll_in.hw, > + [CLKID_DDS_IN] = &dds_in.hw, > + [CLKID_SYS] = &sys.hw, > + [CLKID_CLKTREE] = &clktree.hw, > + [CLKID_RESET_CTRL] = &reset_ctrl.hw, > + [CLKID_ANALOG_CTRL] = &analog_ctrl.hw, > + [CLKID_PWR_CTRL] = &pwr_ctrl.hw, > + [CLKID_PAD_CTRL] = &pad_ctrl.hw, > + [CLKID_SYS_CTRL] = &sys_ctrl.hw, > + [CLKID_TEMP_SENSOR] = &temp_sensor.hw, > + [CLKID_AM2AXI_DIV] = &am2axi_dev.hw, > + [CLKID_SPICC_B] = &spicc_b.hw, > + [CLKID_SPICC_A] = &spicc_a.hw, > + [CLKID_MSR] = &msr.hw, > + [CLKID_AUDIO] = &audio.hw, > + [CLKID_JTAG_CTRL] = &jtag_ctrl.hw, > + [CLKID_SARADC_EN] = &saradc_en.hw, > + [CLKID_PWM_EF] = &pwm_ef.hw, > + [CLKID_PWM_CD] = &pwm_cd.hw, > + [CLKID_PWM_AB] = &pwm_ab.hw, > + [CLKID_CEC] = &cec.hw, > + [CLKID_I2C_S] = &i2c_s.hw, > + [CLKID_IR_CTRL] = &ir_ctrl.hw, > + [CLKID_I2C_M_D] = &i2c_m_d.hw, > + [CLKID_I2C_M_C] = &i2c_m_c.hw, > + [CLKID_I2C_M_B] = &i2c_m_b.hw, > + [CLKID_I2C_M_A] = &i2c_m_a.hw, > + [CLKID_ACODEC] = &acodec.hw, > + [CLKID_OTP] = &otp.hw, > + [CLKID_SD_EMMC_A] = &sd_emmc_a.hw, > + [CLKID_USB_PHY] = &usb_phy.hw, > + [CLKID_USB_CTRL] = &usb_ctrl.hw, > + [CLKID_SYS_DSPB] = &sys_dspb.hw, > + [CLKID_SYS_DSPA] = &sys_dspa.hw, > + [CLKID_DMA] = &dma.hw, > + [CLKID_IRQ_CTRL] = &irq_ctrl.hw, > + [CLKID_NIC] = &nic.hw, > + [CLKID_GIC] = &gic.hw, > + [CLKID_UART_C] = &uart_c.hw, > + [CLKID_UART_B] = &uart_b.hw, > + [CLKID_UART_A] = &uart_a.hw, > + [CLKID_SYS_PSRAM] = &sys_psram.hw, > + [CLKID_RSA] = &rsa.hw, > + [CLKID_CORESIGHT] = &coresight.hw, > + [CLKID_AM2AXI_VAD] = &am2axi_vad.hw, > + [CLKID_AUDIO_VAD] = &audio_vad.hw, > + [CLKID_AXI_DMC] = &axi_dmc.hw, > + [CLKID_AXI_PSRAM] = &axi_psram.hw, > + [CLKID_RAMB] = &ramb.hw, > + [CLKID_RAMA] = &rama.hw, > + [CLKID_AXI_SPIFC] = &axi_spifc.hw, > + [CLKID_AXI_NIC] = &axi_nic.hw, > + [CLKID_AXI_DMA] = &axi_dma.hw, > + [CLKID_CPU_CTRL] = &cpu_ctrl.hw, > + [CLKID_ROM] = &rom.hw, > + [CLKID_PROC_I2C] = &prod_i2c.hw, > + [CLKID_DSPA_EN] = &dspa_en.hw, > + [CLKID_DSPA_EN_NIC] = &dspa_en_nic.hw, > + [CLKID_DSPB_EN] = &dspb_en.hw, > + [CLKID_DSPB_EN_NIC] = &dspb_en_nic.hw, > + [CLKID_RTC] = &rtc.hw, > + [CLKID_CECA_32K] = &ceca_32k_out.hw, > + [CLKID_CECB_32K] = &cecb_32k_out.hw, > + [CLKID_24M] = &clk_24m.hw, > + [CLKID_12M] = &clk_12m.hw, > + [CLKID_FCLK_DIV2_DIVN] = &fclk_div2_divn.hw, > + [CLKID_GEN] = &gen.hw, > + [CLKID_SARADC] = &saradc.hw, > + [CLKID_PWM_A] = &pwm_a.hw, > + [CLKID_PWM_B] = &pwm_b.hw, > + [CLKID_PWM_C] = &pwm_c.hw, > + [CLKID_PWM_D] = &pwm_d.hw, > + [CLKID_PWM_E] = &pwm_e.hw, > + [CLKID_PWM_F] = &pwm_f.hw, > + [CLKID_SPICC] = &spicc.hw, > + [CLKID_TS] = &ts.hw, > + [CLKID_SPIFC] = &spifc.hw, > + [CLKID_USB_BUS] = &usb_bus.hw, > + [CLKID_SD_EMMC] = &sd_emmc.hw, > + [CLKID_PSRAM] = &psram.hw, > + [CLKID_DMC] = &dmc.hw, > + [CLKID_GEN_SEL] = &gen_sel.hw, > + [CLKID_PWM_A_SEL] = &pwm_a_sel.hw, > + [CLKID_PWM_B_SEL] = &pwm_b_sel.hw, > + [CLKID_PWM_C_SEL] = &pwm_c_sel.hw, > + [CLKID_PWM_D_SEL] = &pwm_d_sel.hw, > + [CLKID_PWM_E_SEL] = &pwm_e_sel.hw, > + [CLKID_PWM_F_SEL] = &pwm_f_sel.hw, > + [CLKID_DSPA_A_SEL] = &dspa_a_sel.hw, > + [CLKID_DSPA_B_SEL] = &dspa_b_sel.hw, > + [CLKID_DSPB_A_SEL] = &dspb_a_sel.hw, > + [CLKID_DSPB_B_SEL] = &dspb_b_sel.hw, > + [CLKID_CECA_32K_SEL] = &ceca_32k_sel.hw, > + [CLKID_CECA_32K_SEL_PRE] = &ceca_32k_sel_pre.hw, > + [CLKID_CECB_32K_SEL] = &cecb_32k_sel.hw, > + [CLKID_CECB_32K_SEL_PRE] = &cecb_32k_sel_pre.hw, > + > + /* Internal clocks */ There is no need to make such groups within the table. I don't think such change was requested in the previous reviews. Also grouping the IDs the way you did is a bad idea. 1) Since the clocks are registered in the order of the table, this will make a lot of orphans during the registration of the controller. It does work but it is not ideal performance wise. Whenever possible, it is better to register the clocks from the roots to the leafs. 2) These 2 nice groups are going away the minute you add another clock which was not part of the original submission. This is why IDs have no meaning, no even groups. > + [CLKID_XTAL_IN] = &xtal_in.hw, > + [CLKID_DSPA_SEL] = &dspa_sel.hw, > + [CLKID_DSPB_SEL] = &dspb_sel.hw, > + [CLKID_SARADC_SEL] = &saradc_sel.hw, > + [CLKID_SYS_A_SEL] = &sys_a_sel.hw, > + [CLKID_SYS_A_DIV] = &sys_a_div.hw, > + [CLKID_SYS_A] = &sys_a.hw, > + [CLKID_SYS_B_SEL] = &sys_b_sel.hw, > + [CLKID_SYS_B_DIV] = &sys_b_div.hw, > + [CLKID_SYS_B] = &sys_b.hw, > + [CLKID_DSPA_A_DIV] = &dspa_a_div.hw, > + [CLKID_DSPA_A] = &dspa_a.hw, > + [CLKID_DSPA_B_DIV] = &dspa_b_div.hw, > + [CLKID_DSPA_B] = &dspa_b.hw, > + [CLKID_DSPB_A_DIV] = &dspb_a_div.hw, > + [CLKID_DSPB_A] = &dspb_a.hw, > + [CLKID_DSPB_B_DIV] = &dspb_b_div.hw, > + [CLKID_DSPB_B] = &dspb_b.hw, > + [CLKID_RTC_32K_IN] = &rtc_32k_in.hw, > + [CLKID_RTC_32K_DIV] = &rtc_32k_div.hw, > + [CLKID_RTC_32K_XTAL] = &rtc_32k_xtal.hw, > + [CLKID_RTC_32K_SEL] = &rtc_32k_sel.hw, > + [CLKID_CECB_32K_IN] = &cecb_32k_in.hw, > + [CLKID_CECB_32K_DIV] = &cecb_32k_div.hw, > + [CLKID_CECA_32K_IN] = &ceca_32k_in.hw, > + [CLKID_CECA_32K_DIV] = &ceca_32k_div.hw, > + [CLKID_DIV2_PRE] = &fclk_div2_divn_pre.hw, > + [CLKID_24M_DIV2] = &clk_24m_div2.hw, > + [CLKID_GEN_DIV] = &gen_div.hw, > + [CLKID_SARADC_DIV] = &saradc_div.hw, > + [CLKID_PWM_A_DIV] = &pwm_a_div.hw, > + [CLKID_PWM_B_DIV] = &pwm_b_div.hw, > + [CLKID_PWM_C_DIV] = &pwm_c_div.hw, > + [CLKID_PWM_D_DIV] = &pwm_d_div.hw, > + [CLKID_PWM_E_DIV] = &pwm_e_div.hw, > + [CLKID_PWM_F_DIV] = &pwm_f_div.hw, > + [CLKID_SPICC_SEL] = &spicc_sel.hw, > + [CLKID_SPICC_DIV] = &spicc_div.hw, > + [CLKID_SPICC_SEL2] = &spicc_sel2.hw, > + [CLKID_TS_DIV] = &ts_div.hw, > + [CLKID_SPIFC_SEL] = &spifc_sel.hw, > + [CLKID_SPIFC_DIV] = &spifc_div.hw, > + [CLKID_SPIFC_SEL2] = &spifc_sel2.hw, > + [CLKID_USB_BUS_SEL] = &usb_bus_sel.hw, > + [CLKID_USB_BUS_DIV] = &usb_bus_div.hw, > + [CLKID_SD_EMMC_SEL] = &sd_emmc_sel.hw, > + [CLKID_SD_EMMC_DIV] = &sd_emmc_div.hw, > + [CLKID_SD_EMMC_SEL2] = &sd_emmc_sel2.hw, > + [CLKID_PSRAM_SEL] = &psram_sel.hw, > + [CLKID_PSRAM_DIV] = &psram_div.hw, > + [CLKID_PSRAM_SEL2] = &psram_sel2.hw, > + [CLKID_DMC_SEL] = &dmc_sel.hw, > + [CLKID_DMC_DIV] = &dmc_div.hw, > + [CLKID_DMC_SEL2] = &dmc_sel2.hw, > + > + [NR_CLKS] = NULL, > + }, > + .num = NR_CLKS, > +}; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel