From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753215AbbIRS1l (ORCPT ); Fri, 18 Sep 2015 14:27:41 -0400 Received: from mail-by2on0138.outbound.protection.outlook.com ([207.46.100.138]:18620 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752078AbbIRS1k (ORCPT ); Fri, 18 Sep 2015 14:27:40 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=scottwood@freescale.com; Message-ID: <1442600846.19102.132.camel@freescale.com> Subject: Re: [PATCH 02/26] clk: Replace __clk_get_num_parents with clk_hw_get_num_parents() From: Scott Wood To: Stephen Boyd CC: Mike Turquette , , , Boris Brezillon , Chao Xie , Krzysztof Kozlowski , Javier Martinez Canillas , Tomasz Figa , Maxime Ripard , Emilio =?ISO-8859-1?Q?L=F3pez?= , Tero Kristo , Geert Uytterhoeven Date: Fri, 18 Sep 2015 13:27:26 -0500 In-Reply-To: <20150918155612.GY23081@codeaurora.org> References: <1438362246-6664-1-git-send-email-sboyd@codeaurora.org> <1438362246-6664-3-git-send-email-sboyd@codeaurora.org> <1442535513.19102.83.camel@freescale.com> <20150918155612.GY23081@codeaurora.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.0-fta1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Originating-IP: [2601:448:8100:f9f:2d70:bd3f:bb1b:9b2b] X-ClientProxiedBy: CY1PR0801CA0023.namprd08.prod.outlook.com (25.163.136.161) To BLUPR03MB1474.namprd03.prod.outlook.com (25.163.81.16) X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB1474;2:eotJdqpC4r76WZP1fnG18WLWMcJTGdtCNzH1NqlF4JarrrsBUvD0D5Igu7RrCze5zKPdO5Id+wArm5Ope+k9EHF/rTVDSgRWeqlFCsbNe7SuQ8XzcWcuywFHMAhoomYsFvzIbgEy7atIRbztZ55ZAOzahKldwAmz11Gk53LCzlk=;3:Sjhz+GgcmLm04FK0tAsZYMLbP+VSNBLXaaZ6J7y9g69Dsc0Ff3OAVL0VwME2sxpmFQaeHUOxA3DKwc3qZHVhTrd7U92RldxYY+lEt9GsmCH5RjcWshJEYP/eM20wegJSyqpmjmRerWKadT2C8ZUmcA==;25:edxGNoi9EqzsPEAe5OMQJCM/9HeOVmgmhqQw3pXCi1Nc+Ns956qf+yJO7MS3G7E3ZL92ql5lc8zMSdnxrS0/nhv/aWYD2DM+28xNTMndGT4kq8Zx6uWOyQnfFQYr7oeJ7nmKsyCSIUMEM3qP7yolrCJRBpjHz7O0eDTJb71ZhMQykWLR45xTkjyZ614Stvxc/PfWJhWGo6elCBqyQ8xGWEbjBeIKSQ8SoSGxdBIwZ3iFzbGxealzU0rPaRsJAvzCxPCgkQ5G7T/Gq7osC6524Q== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR03MB1474; X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB1474;20:H6U2AsAxEbXlGtBoXpCqwYhOUe4gjl5Ux7Ix2hADdBxyohXOw6W4/H8Yj5nTz9wXTLUYVpBoomeZo2+WWVgMQ6bfgUVraCj54jyKEETvZJBMD3N1+PQdPZiVqfLZz6EK+9DK5GP3gqKFkgoOgO2armqkm4Yngd3l7dyvsK2P+SaxkJzXRgAi9BFM8/k+FFRRKrntuZqRmA9Al0oPo9mdyiPqCMA6AYjYthd7utdXcbIKW0moHX8qucGxDpd8jxqmfqoTvjwnT78/u5Wriuoyj4eRLfVamGNRbciOCDLeypCyOKg9/RTRmEtXuvLl29n0FAVnlcNpwxc16FB5ya06PKkda+ZMzlYsTn8H5Qq7l5dXarSdrlLjP2xfKpfkZLUGEGdPZIT0ZoO8VzuMo6y5MEJ6xerYCKXHjKKZdGQjzW18GclbX1df82CxOHMRLdRdmKWxzTze5P4UM6cYksa4+O/9qQmY7Tz6UcUECwjmgWQRxatP1b8i4iuXAMhHUNqm;4:x835M5W/0xSwDW3aOdOc+pdQHozLxLjN6Q4Bc44I8Og25KTzKdOrbBdSzwDIbhYyCDgCLd217msZ7hoS5vj+uaunA3Dq0OmbYrxVzg8w5filvaXkALsi68U9cfMou4UjcWCvMWrU7MnpUp9pi4nOD6QLXoVjNNf27fwjFpGFRy5qp+Dy38xC3b72qNjwoi5Wit7bVV75WyzCHroNvkWOO8PsvCyraU9IigasgjKI/3dz0rAYqYWwVC+JXFhvRlMl1S45iNhcPuj2oGaVFgyyZ+4gjm/WW2ESyYM5O+PTlFjO4MEIia6o4CPx0OxJqj0k5jy7KqAlr8sL3vPJ2qNcXg== X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(520078)(5005006)(8121501046)(3002001);SRVR:BLUPR03MB1474;BCL:0;PCL:0;RULEID:;SRVR:BLUPR03MB1474; X-Forefront-PRVS: 0703B549E4 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6009001)(377424004)(199003)(189002)(52044002)(24454002)(50986999)(46102003)(122386002)(40100003)(105586002)(86362001)(93886004)(87976001)(42186005)(33646002)(64706001)(103116003)(101416001)(5820100001)(47776003)(50226001)(106356001)(2950100001)(92566002)(76176999)(50466002)(19580395003)(5001920100001)(110136002)(62966003)(5001830100001)(5001960100002)(5007970100001)(15975445007)(23676002)(77156002)(77096005)(36756003)(81156007)(5001860100001)(5004730100002)(68736005)(4001540100001)(97736004)(189998001)(99106002)(3826002)(5001840100002);DIR:OUT;SFP:1102;SCL:1;SRVR:BLUPR03MB1474;H:[IPv6:2601:448:8100:f9f:2d70:bd3f:bb1b:9b2b];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTFVQUjAzTUIxNDc0OzIzOjRJMGVkSEx5TklYWVNjSTYyNVRzM3EzOE5w?= =?utf-8?B?V3A5KzR4TGVZdkZGNzFiNnVKM0VhekxQbDZVbWdWTjFsNG0wS21uRWl4MDZI?= =?utf-8?B?cy9UMHAvbGRuNzhYSGxySEdNckdWM3dKZVFVOUt0ZGZDSlV6cFlVYmhOL2RN?= =?utf-8?B?L25POTBlTzF3SjZ4RFdTcnRzU2RzYlBuR1VXdmhsd29uSFVYS2tyUFNOSkRC?= =?utf-8?B?V2lNUVQrUEFkdDNhRHRJSWdWV0ZMbHNmaVBKWDVMVlVFSmRRdW11ZnRYVUpQ?= =?utf-8?B?OCtacEJtNEZBV3JOVkl4a3AvRCtrUEdNUkVlSWFKdFFzOERZY0IzRXhhcUtW?= =?utf-8?B?VmRjcGtnMDJXNHdQMDN3dzgrUXRYMnc4SkxkQWRydkh4NjV0d0g1SGd6ZUUw?= =?utf-8?B?TmpkcENxRmtpWnAxOFZ6N2QvaHNyMXlPT0xsUWRUOTFEU0JhQTJPR0RPbFVk?= =?utf-8?B?QTdrQmhyTk9Fb2xmcjY2WXI1U1ZQcmY5ZUIydDRvWlhxZGVkYWlwcko3YURL?= =?utf-8?B?Z1BybU1jeUk3cmtyOXI3bU5XcHNlcE1nUWJwdnRMbFd1TmlGVEkzZDBTTGtn?= =?utf-8?B?Umg3dkpwTENKaEFnM0lIUE9KZW8wZEFNeTV2UjBsbllqWmFaa2c0eVRiNHpM?= =?utf-8?B?SVdDZ2FNWXQxU0FhQVgzTFdVZGEya2JURFJIcEVwZUhrRDN3OG5yY3B6b1R1?= =?utf-8?B?TjRmeVpwc0R6S1RGRFZnQ09ObEMxcW9ZOHRZaUFHSmtONFZBdkpUTUJZbW8x?= =?utf-8?B?ZGthcm1TZE5oMzJ4c1B3YnFzeE1CMFJ6eGFaLytNV0hMdFROZ3psTXFMOGNT?= =?utf-8?B?SzNlME5HZUswd3JzSmgrZTJxVlFndTYyQ1RqaXZIQWFGN3EyQ0IzMmgyN09t?= =?utf-8?B?clhlMDNhSjlveWY1bVYybDRUWVgwS3ZJa0owMWtpckpsbGc2bzdvdm9UYUow?= =?utf-8?B?QVRKN0VwTURVQ2NpWTY0Sks1UWZwVFVHOFNnRTFoZFlzeS8rTEl2d0tzeXJ3?= =?utf-8?B?eHJaeWJFeE9raTFjS09rckxiYTI2TzRmNnpaaDQ1aWNKLzJLZTV2ZnFqQXV2?= =?utf-8?B?QlJXK05KSkI0S3RPNldqU2FQZndpbHhCVlEyQVovc1RhdStiVjhHZWtwdXV0?= =?utf-8?B?d1VINTdyaUVrYWFtL3hGMkkwZ1E2L3FMMUhVL09weXNCUHo0cW8vUGRXcmYx?= =?utf-8?B?WlhUU3NRL1BidHR0elVqM3dMeWp1NVVucCt6Y2k2Vkc2SlA4NW5GQmc5bXVX?= =?utf-8?B?SWZRcHNqaFZPdXc3b3BPR05yWmw3TWJJT0NsS1MyY1JkYXlyNEdPVzloSFg1?= =?utf-8?B?cVNSMllySjBQNW5waGNQSEhxZC9hN0FKZmVlL2hKNUtyOUhwTUpZbnQ3dSs2?= =?utf-8?B?MkVYa3Q2U0tuVFdBZEN6eEw1dk83WTZ0RzVMeGM2bEVMS2I4YmtaU1h0empS?= =?utf-8?B?bktIN0FJOE9oRXUrajVrRnFSSnZMWFhGRUp2TlZ2dnZkVzl6d05wUUJmRHBw?= =?utf-8?B?ckpKVHpWYkJCRkNyNFJ3QTBwYXN2T0RCYiszaFpnZU1jejM4Z1FydlNOUjcw?= =?utf-8?B?Q2lBajJkVjYyeU4rUFZNQlh1ZGVLWlIwK1VtczhkRDZSY0J2dVFhMUxyVXd1?= =?utf-8?B?aGQ1UVQyN3VmbXpBOEg0NjJ6Tys0WloyUURuZzJxVDUrYkRHZ3YxaDhGYWpu?= =?utf-8?B?RE80L29ybk1SdnYyem00dWRuQzgwWm9idUsyelBEWitSMzVWUGlhSnA3TEpG?= =?utf-8?Q?6DA3BbKBIz0Y/Xnsw6vJWYlSjxivhHkCiOxQ0=3D?= X-Microsoft-Exchange-Diagnostics: 1;BLUPR03MB1474;5:MWLXF+uk1eSapEjHt3pVe/L8EnZx9uj/R20UPHg0CuRRDrSIVkKz48J0SPMuxzAbqtWBrX1M+nVD08phvVKe6R/677zhgRQhxIit3m0zRP95FNCC/n0jd7w00HkhU1dCofOJGFHf3Nd7ARsiPzQf6g==;24:3Dx34GDgJrR1ZWFTXbsPktpoZiDUpzDjDEVReXoUACp674x2BJJx3cvRl4yHQsXdMDV67MIxRzlYw0/hrAwxgOatFpY3rwpeNsdPzV1vzn8=;20:Ta55tZuk862ryLsZx55M8iAmgP5S8AkqUWOBl2cnhmgrx0lvlOFcwiuqpbzNfyiHaNuXq7GPU1tPkOZtdJELeg== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Sep 2015 18:27:34.4711 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR03MB1474 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2015-09-18 at 08:56 -0700, Stephen Boyd wrote: > On 09/17, Scott Wood wrote: > > On Fri, 2015-07-31 at 10:03 -0700, Stephen Boyd wrote: > > > Mostly converted with the following semantic patch: > > > > > > @@ > > > struct clk_hw *E; > > > @@ > > > > > > -__clk_get_num_parents(E->clk) > > > +clk_hw_get_num_parents(E) > > > > I don't understand why this is considered a clock provider API. How is a > > clock consumer, such as cpufreq, supposed to find out the number of > > parents > > or similar information, so that it knows what its options are for calling > > clk_set_parent()? > > > > This is the caller I had in mind: > > http://patchwork.ozlabs.org/patch/507619/ > > > > Surely asking the clock to describe itself is better than what that > > cpufreq > > driver currently does, which is to look in the device tree and make > > assumptions about how that maps to what the clock provider driver does... > > > > All the APIs that were converted were private to the common clock > framework and in clk-provider.h, not clk.h. clk.h is the consumer > API that's supported by more than just the common clock > framework, so whatever we do for the consumer API needs to be put > there. I realize that's the theory, though it didn't seem to be adhered to particularly closely (e.g. even lib/vsprintf.c includes clk-provider.h, for access to __clk_get_name), and indeed the entire way the API and struct are split seemed quite odd to me (and the out-of-date Documentation/clk.txt didn't help). > For example, we recently added a clk_has_parent() API that can be > used to probe for possible parents of a clock. Perhaps we need > something else in this case so that consumers can iterate over > each parent of a clock? Feel free to suggest a consumer API. OK. The suggestion is that functions which are potentially useful to something other than the clock provider itself be moved to clk.h and operate on struct clk *. At a minimum, I need "get_name" and "get_num_parents" (I'll add a patch to do so on respin, if there's no objection to the concept), but some of the others are probably reasonable to expose as well. > Is there any reason why we can't use DT OPPs for the code that > you're patching here? At a quick glance it looks like we could > leave this driver behind and move to cpufreq-dt.c and then use > OPPs to populate the possible frequencies and affinity. One reason is that I want to maintain compatibility with existing device trees. However, even ignoring that, cpufreq-dt doesn't seem like a good fit. It requires specifying voltage, which is beyond the scope of the DFS mechanism on these chips. It also requires assembling a list of valid frequencies in the device tree, which is not knowable at compile-time as it depends on how PLLs were configured in the reset control words, and in some cases the revision of the SoC due to errata -- and even if that information were constant, it would be extra maintenance work to keep the redundant information accurate, and if errors were introduced into the device tree we'd have the same sort of compatibility problems I'm trying to fix with http://patchwork.ozlabs.org/patch/507621/ -Scott