From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753123AbdASOx7 (ORCPT ); Thu, 19 Jan 2017 09:53:59 -0500 Received: from mail-bl2nam02on0087.outbound.protection.outlook.com ([104.47.38.87]:33056 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753027AbdASOwp (ORCPT ); Thu, 19 Jan 2017 09:52:45 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Jan.Glauber@cavium.com; Date: Thu, 19 Jan 2017 15:50:41 +0100 From: Jan Glauber To: Ulf Hansson CC: "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , David Daney , "Steven J . Hill" Subject: Re: [PATCH v10 0/8] Cavium MMC driver Message-ID: <20170119145041.GA11757@hardcore> References: <20161219121552.18316-1-jglauber@cavium.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [88.67.142.24] X-ClientProxiedBy: HE1PR02CA0081.eurprd02.prod.outlook.com (10.163.170.49) To CY1PR07MB2588.namprd07.prod.outlook.com (10.167.16.138) X-MS-Office365-Filtering-Correlation-Id: 0aeffd0d-2179-405e-89bb-08d4407a91b5 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:CY1PR07MB2588; X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2588;3:nJmBb0lcgaSroQlfO3+B0T5rTZPGY0CdQFLTDnZEAEW1/Xjd8WiOVDu0xLq/CUVfpa/mwawz2TOgsxpDrc8jj18B22DEhu76uTIPjxdSekea8bVfYBE/Si2tKvDWzIClf1gdrx9qw3L/yBo57PadeY0JUtkt9Jsd1V7cDqjpzTdfil0u2Diz9cSDSpQbAbCjhMqQBGmqJQhwdqxUibLgyKbe6u7OWLCvqkIo22GtdfSkWcAirp/+DOCdvdeBaozSXdzwumUr1GwHsohHWzAHjg==;25:PuJeuBCxfDz8r52056eSsysZIhEcjw8Aoti0sphUQA/3E95mKx9syD1ag6TVqfbwa/wTxwMEpmKOHSfoJhec4w+ACbiP1TInblLK9H0tX3Wd05VUhTEWQZjECIAMhZQOrChKexGh34G/karWqKX9+FP9DRm047YPazyJ1Bb1s5VUhYdBpJDq0hlxDEIjagyADTZvbzxig11mkDtrF4ib/AY0lm9TS62DzUpgt3igLpxV29ZGYmF8sdnfPixD9QmPxM4f5oAv5wIjz3Vpjtm0gmhWbBIBYNTsTxtPvDf6PsTvdpdXz4V5D0GjxRNU8IatTc1gSc8HwOc4CyGobF+gXOGrpiGoiYyiaN/4HCxGp0JV72E3Ri6K8/WKoU0xrxllETEU72AseEPCMFUH4yL5l/WGAK/MnrcCUzXpPLDpt9Gw8u5MyGGYmHfjbE/1oZAfLEuI6WU+j7Fh+yGrj5FQug== X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2588;31:BH15UXNIbIH0+VgalZhZI04SYNyJMo+1/y1He2pFU/iLRr47K5EFVD/8aG+1gjeooqa8W0uHhgxKEHMusNdlqZmnE3GYMlOtqNZ6V4hMyE8Sm7aAb4ZDgjm6xswC7Ifu7TmZ/HZ/0Ho/ulaX56zD/3wUy0uZHFuvARLA4IvoAkoylbh0XW8eWfqKOYBKcuuw9/sSONsuJr9YFYGmuMh2PM4ELxSvh67R2JFkkS/mQli1oiGd3aFmOi3ANLk3Ryy5;20:5mWdnlhig3WTrupZLYJ301+YN0RTNeWXFS3RElVLixVUfpJR+P60UTY+PQW3Bv94GdO3IfUA2N9eE/NY4STDHSmYXXdkPjzaVok0Nd1MxkwFYFo70pgivSPpqmayqtTZycMOX7+3hd1vVICrseTwwuLgMkWXXwenKN5eFIuC20gSlhsa6szNDMDkOIYMshX4RYbTid5bY7k3AvNGBizRMQIdvd2jHwuSPFjq60jLUmdGWmv4Lvcjr7tyUgzX/sbukGF8PQ/ls6ErjdbKdZ3p18xSDNTPU0HiBnL1r2v7Ct1c9LbaVEFMMuYKhqlzz21tBJeURHcwObS7Kfdl/KE5rfOzdNL91K81qigfkVuoUQwsExj/0HhZCCzM9x7NMMDa5nnzkAxf/uhegRHaF2yUKcgRo6VnWUovkOUCR1FjsUhhakM7aXo+YUTWqttZ0otzTS7DDlLjtIyA1FLVIAzWY7DdhUJQ3V4MW1XfrgYme8zN+bQo9Lt1V9/rWEUCTd1mX23m35D2tf4EyRUvl+X0Na0XHLGxnab3FRk/qaOTrRZgsglhx1W86azpDDVG6ugtPGF1aSQzsNDRk15g+YR4/vq4/uvamsPMgv576KUKwxI= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6041248)(20161123562025)(20161123555025)(20161123564025)(20161123560025)(6072148);SRVR:CY1PR07MB2588;BCL:0;PCL:0;RULEID:;SRVR:CY1PR07MB2588; X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2588;4:0s+5qSKGwq+qNWd19SQvY15uwE7O71VpYy97qp7eTkB13zgxQaPFJBYVdwIVfM7LoYuO9l9YXvlpxLnOUhPgGkcR+carFM6GPnU1MIpkW7cIyZGfqbluSYqAw082Vr8+Hfx2FqN6l7JcxWLyJ9Ts5diS2ceo9YogQNyHZBg300OD84VGa/EhD32UBRu2KsDxL7dHzGUraceqx2RLgeUmNGhArlUyQuJMZNIHo5Ihibl4U0xVLombEvQBQoUHLtt1nHwN9bU6xDiyaXVs3fBJiu3Ghh8duAO7Z1a2D5enX0dLBiwYvCorRPiC3P1sbEMPr1acKQPklZqshDBZ6oeyKibxTvt4TEmkI4wrOPlqDcuXPYCP/cxZkLn3Qb+p0lHzVYa3S2cccZ8REyUkV2XwIlZoUHKpcYgHMiOc8U75da0F53kHRknzfknZ6gXLDhKkfOPmltUiR3Cb1bBucu1QyYGJKa/bP+hlk5vq3M0kq7FYtiZoMFJ2shsux1RKkRTUbcnzeGHHwi6qF9as02fvNn4JQ/+L7+zGpr7ExJbvjrQYkJg6zUVVUNrC+Muocnhb2bvhbM3mz8bbscPuF+nf4w== X-Forefront-PRVS: 0192E812EC X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(7916002)(39450400003)(50834003)(52314003)(189002)(57704003)(51914003)(55674003)(199003)(24454002)(81156014)(81166006)(106356001)(97756001)(8676002)(4001350100001)(1076002)(23726003)(6116002)(38730400001)(33656002)(6306002)(105586002)(50986999)(9686003)(76176999)(54356999)(25786008)(3846002)(229853002)(6496003)(42186005)(55016002)(83506001)(4326007)(101416001)(66066001)(54906002)(2906002)(92566002)(4001430100002)(110136003)(6916009)(6666003)(46406003)(2950100002)(50466002)(42882006)(107886002)(7736002)(305945005)(47776003)(5660300001)(97736004)(189998001)(53936002)(68736007)(33716001)(18370500001);DIR:OUT;SFP:1101;SCL:1;SRVR:CY1PR07MB2588;H:hardcore;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;CY1PR07MB2588;23:4esra2f0o+KW//FGg0cy6MP9hJDbpLLSNnTbM7Nap?= =?us-ascii?Q?0nGUzhjR27OuidnvrS3TrNqQZWGu4nWDLqsgG4Pfw0GyH+DhNf2KZNprwPgV?= =?us-ascii?Q?AeGGTlY+GVC+WbQ6l2h12zDwegngDJDDTIJh9a71QHuAREvYD6kqB+L8F6+f?= =?us-ascii?Q?aZbwUYEpijTvpP9OF8+g8/zCgxebD6o/mk0J0ONHTjCjq2Kzqq7KrlLye6qL?= =?us-ascii?Q?Oxrr63b92UbIg7mZHuXHBnPSyQEkzXJqhPqts6EL/7l8VgZmhOb9VLre7TKV?= =?us-ascii?Q?2RP8VtIf/GamKHbrGuyXyNN9e98GhUx/xVimdNQrB7aZ9w58hpRRWsgKS8Lv?= =?us-ascii?Q?qs5WlgfOiOcQu2Kd2bKIs7lS4h3VRwm486SGvB2+ZB7nNOtolZ6Nb+J35dWQ?= =?us-ascii?Q?wogwro3uga+WxoXIfGhGUF0kRNxDYFiWFEEwgXjxuqxswffEZm9we4hJWUSG?= =?us-ascii?Q?iGAIeWlxQT/StsuUYgYacidIxE/aaQIbXRD/r9FfdiZZrVbDfo0fmhgtcG13?= =?us-ascii?Q?4FSBXvI8WuwC5KUNGwGhxjGGFhZ5vxMIh7vji6GBN3HhgTiWC5NERlvlau9n?= =?us-ascii?Q?azxS3yHoYoeT2ju/yJpca/FJ5mCdzq4feZXZ+pGefsdLB4kyDxm+X8JFqAor?= =?us-ascii?Q?Knxikz5uPu4okUWpBo2bECKJlZDOcKpiNVrUTyL+71t1qlsbLqfsYm4A6rrs?= =?us-ascii?Q?sG4w4H+wZKuhuhPsQ8iqqGn5kzSm/GcPxnz5r1O41kksHrB9wrep87aPS9CO?= =?us-ascii?Q?EdrrE9xlSUZtAPp/auF2VEazmiYHS2g/54TM6uxm4u40l2WgwNRyIzWNxUSq?= =?us-ascii?Q?AA2ZtmkdlgGmCXZY5YeAfdSPs0owyKi/e9CysUYjS4tGkQATfhMB2bV+dvMe?= =?us-ascii?Q?rrWOkOdnMZ1NcMvwbhEbPtoRZmdWLyxq9R7qlxZY3gzDttmCDufnnn+JT1Vw?= =?us-ascii?Q?QCAFOpxm4HsamgaR2NsP3+BUPb9Xd9IEP4rqYJbtvlVuaYG6DzYUXfGAoPGa?= =?us-ascii?Q?koMT1wQghYiubcqyE1m8/kPIf5nB9wHUczFL6VDwYLtlglgAul2zhF5Xr1Jc?= =?us-ascii?Q?lXXvkEh0oZqHHaMfI022R+OcDWMzgUK/hKPuJqnFPseL9ZpdhP7nBiKagQO4?= =?us-ascii?Q?QP/+N5OhW0BGfT+pDLHVjvOSkwUW1JDPaZRjN2zLwoE0K2vkQ6Em0S05JSWj?= =?us-ascii?Q?jaCqe1suidtKK1MgAehOmTiMLpl7gmrvN1FWkNOMDY8umYtVKCsgsiAduhKX?= =?us-ascii?Q?DxrhtrX0atoM+NeM8Lat6ixL+adCgO5Mr51bLhlEJQdnPYKV34//nTzjZuBe?= =?us-ascii?Q?2YrGFFL2mlYV/gTqln2k6+h82IDuwp2V62kmZO10eGNnGdyvaNU4KtI/4Pb/?= =?us-ascii?Q?HYAbj5rIgQyHI0lmKuv5JckNqIsugP+WoUaXDwUZx6hH1RAcMm+zLAwcIXQQ?= =?us-ascii?Q?9cOU0zA1TD3vJ8UBD523AQyP7qhkApwV9vCDbH232L6xTQoRy2o?= X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2588;6:H1/1oxB2Z+aQX6VYmAvZfOsCMK6qQdv1hQKuAfVEyL+bJB/AMM90M0/chy42XRkSRkLvFMrtUyOYX8ojnEjZRiYBrQ0SH2vNDEq+uRv7FzzyJBg0Jipv8UjU7LzwuWnBz5XXkOjCXbkp3twTE8Ps4kdvP+jHPeLCwq7tVQRI+Cpb1DZq3gn6mXeYheie9d4vJWVfm+NE9oei3Q/nMqAr+fyevN3kjZ5r650fAL9E2OMu3lN2fDsmgiVYdbOU18zpd8SKMsZVOWkTQg7jgcQPxgqBaTzMXyq8EXlga3HlQJyq+MdmlSjd1zUycs7qiT/hsVLzKq2SoVyztlF61v1I7DoRIqzIswtIkFPEchllhX1Kulm7jy4gFAAEv4TBZUYIblQyalxlqZsqOTFoLwa9VFQBDPNVfe5cakHFBXhVsyo=;5:sE5NxycNPnTVxwww+VRSIBzTNU/C/fDA6OGPRfmQni5F4xIcH3o+mr5goHVYC0m0PCZKd2dPWjJNlaBE0x5OvSMEC+0GvdqwvFAkmdQ/riCfECiwroRPMkuIXUTOcyV/lneGDmhBPW9UC/UOBcMqQUk48qZF1IIWv111Ce2GyXM=;24:A8v2UtP1HaDYuQR4RRlxBLigE2uEOq8eo0cJ5GFtSYGmDrvJzQnN5H8kTHeNjOc4JTIDETG1kKBqSXcNCFSjH8L7C/ZEzYsakaP99mnCL8M= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;CY1PR07MB2588;7:myjT99omvTGyDGRO94tfbZa4epwodalMCcKbcjIhRGLrIfV6Fsd976PcMQBumTqInhAKLndenwA/ErMsZrb4RcWzVkDS6w5W/zafifkm6Y+jICMaoiDVa//TvKG8OkXhu/XkNuuyEunH9950onumbFxBLAv1fhOe1LGe87yBWxFo+NhVFJvrvH1gSRhssbyV5vD1cv6FjKIchosOqs3pwHYB2jqMFfX0ZHy9geVzUVFm3ntY58zGHuTj2/VyJj4ujFYkOPxlFcFwPsg8NO2mdbu16Qs9V99Nk1yft4r7drK8p9ql9IIp1xczB9RJBnemq4s5+6OtF33N16n/xLn4pi7FzYCF7ODOJIkMw2ggPifDJ8+RzVYdvFEBkObmB/G+RRQz5A59WY46Cy9OiEYmI7cscV4mll6hXb2AkgVj+wn6U/SwfrYtiHglYcCgXbTmHt0plUFsb6+ZiRCwnDEpCQ== X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2017 14:50:52.3919 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR07MB2588 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 20, 2016 at 01:10:56PM +0100, Ulf Hansson wrote: > Hi Jan, > > On 19 December 2016 at 13:15, Jan Glauber wrote: > > While this patch series seems to be somehow overdue, in the meantime the > > same MMC unit was re-used on Cavium's ThunderX SOC so our interest in making > > progress upstreaming this driver has doubled now... > > > > Glancing over the history of the series I think most of the high-level > > comments should be adressed by now (like DTS representation of the > > multiple slots). I've added some new features for the ARM64 port > > and in the process re-wrote parts of the driver and split it into smaller, > > hopefully easier to review parts. > > I only had a quick review, but the overall impression is that it's > getting far better. Here follows my summary. > > 1) I intend to especially look at DTS representation for the slot > nodes, to make sure we have a good solution. Allow me to get back on > this. > > 2) I don't like how you have named files, as it doesn't express the > obvious relationship between the core library and the drivers. I would > rather see something similar to dw_mmc or sdhci. I've prefixed all files with "cavium" and adjusted names, incl. Kconfig names. > 3) Related to 2), I would also like to have a prefix of the commit > messages which express the relationships. Again follow dw_mmc/sdhci. OK. > 4) GPIO powers should be modelled as GPIO regulators. I believe we > have discussed this earlier as well (I don't really recall in detail > about the last things). It gives us the opportunity to via the > regulator framework to find out the supported voltage levels. This is > the generic method which is used by mmc drivers, you need to adopt to > this as well. I've added a fixed regulator to DT: vcc_3v3: regulator-vcc_3v3 { compatible = "regulator-fixed"; regulator-name = "VCC_3V3"; regulator-min-microvolt = <3300000>; regulator-max-microvolt = <3300000>; gpio = <&gpio_6_0 8 0>; /* enable-gpio = <&gpio_6_0 8 0>; */ enable-active-high; }; This seems to enable the gpio. Is this sufficient or do I need the gpio-regulator? > 5) Please reorder the series so the DT bindings doc change comes > first. I need an ack from the DT maintainer for it. OK. > 6) The most important feedback: > This driver has been posted in many versions by now. Perhaps I could > have been more responsive throughout the attempts, I apologize for > that. On the other hand, you seems to have a round robin schedule for > whom that sends a new version. :-) That makes me wonder about your > support in the maintenance phase. I hope my concern is wrong, but how > about that you point out a responsible maintainer? Especially since > this seems to become a family of Cavium variants, it would help me if > I could rely on someone providing acks for future changes. Would you > be able to accept that role? > > > > > In porting the driver to arm64 I run into some issues. > > > > 1. mmc_parse_of is not capable of supporting multiple slots behind one > > controller. On arm64 the host controller is presented as one PCI device. > > There are no devices per slot as with the platform variant, so I > > needed to create dummy devices to make mmc_parse_of work. > > IMHO it would be cleaner if mmc_parse_of could be extended to cover > > the multiple slots case. > > Yes. I agree that this make sense! > Seems like we could try to make use of the struct device_node instead > of the struct device. > > I will try to come up with an idea, I keep you posted. > > > > > 2. Without setting MMC_CAP_1_8V_DDR DDR mode is not usable for eMMC. > > I would prefer to introduce a new cap flag, MMC_CAP_3_3V_DDR, > > if possible. Currently I need to add "mmc-ddr-1_8v" to DTS, > > which seems odd for a 3.3v only host controller. > > This keep coming back. Both DT bindings and changing to the mmc core > has been posted. > > Allow me to help out and re-post a new series. You can build on top of > them - I will keep you on cc. Any news here? Can you give me a pointer? > > > > 3. Because the slots share the host controller using the > > "full-pwr-cycle" attribute turned out to not work well. > > I'm not 100% sure just ignoring the attribute is correct. > > The full-pwr-cycle shall be set whether you are able to power cycle > the *card*. So this binding should be a part of each slot/child node - > if the host supports it. > > > > > For the driver to work GPIO support is required, the GPIO driver is > > not yet available upstream. Therefore, for the time being I removed > > the GPIO dependency from Kconfig. > > Is this is about the GPIO powers or also GPIO card detect? > > Anyway, I am fine with not depending on GPIO Kconfig. > Meanwhile the GPIO driver was posted here: https://lkml.org/lkml/2017/1/6/985 > [...] > > Kind regards > Uffe Thanks for the review, Jan