Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751371AbcDTWnM (ORCPT ); Wed, 20 Apr 2016 18:43:12 -0400 Received: from mail-bn1bon0057.outbound.protection.outlook.com ([157.56.111.57]:9112 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750998AbcDTWnK (ORCPT ); Wed, 20 Apr 2016 18:43:10 -0400 X-Greylist: delayed 884 seconds by postgrey-1.27 at vger.kernel.org; Wed, 20 Apr 2016 18:43:10 EDT Authentication-Results: the-dreams.de; dkim=none (message not signed) header.d=none;the-dreams.de; dmarc=none action=none header.from=caviumnetworks.com; Message-ID: <5718027F.7050707@caviumnetworks.com> Date: Wed, 20 Apr 2016 15:28:15 -0700 From: David Daney User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Wolfram Sang CC: Jan Glauber , , , David Daney Subject: Re: [PATCH v6 10/19] i2c: octeon: Add support for cn78xx chips References: <878b7035b447b6700abc7ec79c375df090067ebc.1460387640.git.jglauber@cavium.com> <20160420215218.GE1546@katana> In-Reply-To: <20160420215218.GE1546@katana> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [64.2.3.194] X-ClientProxiedBy: SN2PR07CA007.namprd07.prod.outlook.com (10.255.174.24) To BN4PR07MB2130.namprd07.prod.outlook.com (10.164.63.12) X-MS-Office365-Filtering-Correlation-Id: 9c6499ba-c87b-42a3-bd29-08d3696b157a X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2130;2:H86LQ4z/bQ4TPtGm9lsmQs9XQMRoGGGRUcogORxAO8UAqM2E+XficEsQKl52zOGYEoUgMXq3/u8Wel7MxAhEtVoV3D3SZIdSSICysI9ORwyql5rWq04BFQRZClNQiASRiuIS75nQN6uxiG/KIKrO6iplGooDY89lYm8c40hRzP22qmjc7BAOjevFAoYC/RvR;3:uhUlXmEHLtcnZASiwdSTNasMBCfYK+K7wbEajbEiJl7W1S2j6dvlCIz0BxX6/Hqz3aPMI26IZhqM3i7sPYEkvsvoLffsNCwaq6HpjhDqHWI788EuM2AmpLGIqXdC7vUg X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN4PR07MB2130; X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2130;25:HiiFbv7lpOd9TD0XM/n4yR+2+tgoVXkgEObh8QZgm+AC5N2lXTptAHwYDg4Cz6yYsLRYflMFY4LhTlk/FgNw+wlNXess+QLyIVc2wvYsdNtLw4goYjFBmFCG7SKaWVnPNISClV8GrBD+medWxPy4BJetix/+1YTq0kd0FJsybQ8pbPWkjW79BLnLN4iEW8CNdL5bseH0TSav65SPv5qz2jm0rLK65qHNi6tyrKX6o5RBhui1w9AA1RP0TFCSqPiA8NMHkdwmDFU/nzLILnBz5VW6zv9KTAkKa/h1PWXOWOHwcbBFmGEdm4flRPZ42y8S9XnC8HRy7NSpcsIWe79BBhdXRtwQGwO9D2P26YqBSuOL3Y58GqcBktHPSiyPmSfgZPqBDDnbn6JmUX02sjarxbQjwO/CLatS9mj5JJPMetHLCP8JDHVhkMBCWYZ27Hay+ry/VVJYSLnNV+CPWtRhYZZb4cJKWAps9scMq204ZAVlPwhw7S3JYGPTis7iP1NhoiOSctLKpzfnq6yWLR4Y3PKHbxD6TY/NvNi6OhU0AffjNgcQj1vSMcDlAL2y0Jt8ruSyOmJrNUPpu7I6LDWoRtA5ezn6vpVIeW/C97Oh9F2CQ265pEWbEcNvuBSnx3Vlp9jrga7thnJqLrTs4wr8TQ== X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2130;20:UrppojVBpXjAf2YJZjNWKeCW6XM7E9LOq+pfwr1UR8t9Y1n4LEwe4AAZ5BzM/sG/AaEKYUBoJUY3g/Qs8+l8J7NeyrWkznqNbIOzkl8VvV3EAa0akJxc2WMuMb3vMH1Fkem9IhnH4vj6AJk44JeXem0AFRYeOCjkosDupSXUBVCoN+Bh4XUa9LLee6+XqbK+ZsH8QtKxY49vIC1fOeWsU8V1l4oEHjNOJfhBAuaUw1rkAVgfkOQnJTTjy1x/i5RPSNdia5fMl3kIZcLzRhHLWM18Qhce1DjYd5GlyXnYpTm/0ECEdW8rm0A0CDlLVgKojkyPrqVrlX3zblha5MfFfn6rv/zOP/sHVmmuswQmGGwJdIppK0zqJ8PwxYiLiTpxirLFUSQUvJajQBBCFpT5I9jjh7y3yzeFj64pT8aPluwkC1Cz513mgTH4YbKSkzL3LesNx3/wcJ3gA0VX354672MIn+XUCy+HU6d8sjbezY4OVnpwnY/WXcbeERvRxTJ3i1C5xzin85S1cel22vzne81oE5Kpc3eVZMwJXEIYZ0TFutn6mC4QrCvywI+qA5TOtI4r0Ctkjgya8U3cCuEl80YmjJk+IFckcGYUylDQS5k= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(9101521026)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046);SRVR:BN4PR07MB2130;BCL:0;PCL:0;RULEID:;SRVR:BN4PR07MB2130; X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2130;4:5KXSU+gowveI8ZVeG5F9N1nGZMCCbrzqDVOTAdjI98PzThjh9o40dom2z5IXvqQwuj+yuiUuKE8Wqe0SgKqp8OCnK8axmMnEyHeulfSjg0T8rgciPySoiWnlkD+QD1dL9vfmSexigDzNAMFPx0/DhyoWBzcHU2ikFQLMEHJkVtdhKB4lMvhtyPqd8PlHvZ1IS8QxXXtUuEgdXe7FAVUgOg1GODK2SVqV67iTWcQnPtkYpcXxs8vyYZDC0dGfOqroEiK6L5OMVeMMYxfj93FuXdRkFp0DiZZ0BixKh8a9FHDxWNaYGJnz2VLXlbwqfYuzQAiwgUVoCwKC4VI9e8aqzapTtKy1x57MoL4lDdhfIfQf1dokcXEbNNU+zEWzXSsmbCcNyYW2QEXyd7xJbhMF/w== X-Forefront-PRVS: 0918748D70 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(377454003)(501624003)(24454002)(81166005)(50986999)(36756003)(65816999)(54356999)(76176999)(87266999)(2906002)(5004730100002)(92566002)(4001430100002)(230700001)(80316001)(19580405001)(5008740100001)(42186005)(19580395003)(53416004)(4326007)(4001350100001)(66066001)(47776003)(107886002)(110136002)(189998001)(586003)(2950100001)(6116002)(3846002)(77096005)(1096002)(65806001)(23756003)(65956001)(575784001)(64126003)(50466002)(33656002);DIR:OUT;SFP:1101;SCL:1;SRVR:BN4PR07MB2130;H:dl.caveonetworks.com;FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;BN4PR07MB2130;23:bsLLQuJIJBKp3FqlkLUfGajU12E7cmaMnUlluKd?= =?iso-8859-1?Q?IorPEd1c8nq0x4utqO8iz8LFoNSCNDouKGgZrCm7guBzTjXVSX3wyiTHWZ?= =?iso-8859-1?Q?FKnY2U1Q3ocOtWvnCqxMP2J3cGei+E/vrbxvnArtij1p+RKlfWTbbPz/e6?= =?iso-8859-1?Q?sGKsljRUu4+cIykYrh+l6iHZS0jL4WmN5ommMAqcKYbnAmHJx/VDNm2aIz?= =?iso-8859-1?Q?XtuIG8/70jMAPDx3u2e1MeOYEc4DrBKL62esOyFppxIaovaenFbBa6fozn?= =?iso-8859-1?Q?ygQu8Cq5bZlTmxFj4wYNTgMn/en8v02c74gf2xdxkfmQ9sufD6lSUFYT1e?= =?iso-8859-1?Q?YXom/u+BWtoDPds94EdrTonjarlVQBtW2NzjH6i5ecmz05CTfhXJTvxgMR?= =?iso-8859-1?Q?5Z1lcgsZsgbZy0CaNi4b7M16zDpNUXIcuMESqFGAsGeIwUySTsvI9SIaCZ?= =?iso-8859-1?Q?BjvmoMQaLgm8/UuiWie4mSwiBldTcIKcb2xTZFQicGo50atFTtlHUq1cBg?= =?iso-8859-1?Q?Aomz8sGuQhftcGEwohYKAJlnL1JmFbBxt+i3myxKUAzs+dlq56L3Emrx5f?= =?iso-8859-1?Q?/UUqGfvgFkV+L7d5vSEHYQfDeo4Yksb4KQtoNiNNV7kN1gaFAonHsQX+l7?= =?iso-8859-1?Q?ClGcibdlxnWprjYljSjOQoj9rczjHCnHw2rUEg8mqDTSiNqEf3zNFb3/zN?= =?iso-8859-1?Q?a+uYl4xuYM3cwT/dGoQ8dOQQNwzGWXvMJrm3Yae5g4Evr84cRAjmBPYD/g?= =?iso-8859-1?Q?4SdIBGspARaBQkuO3xMg00SkdVsnTHsJEJdoQHywECiT66jKy66cmbhnfI?= =?iso-8859-1?Q?nKqyuiAbeUlSEhzYHtiZ1dtzIwoZ6nPCdBhVBMwlkksYbLg0slpD286hjr?= =?iso-8859-1?Q?ILDYs/99hSuhZ3WYi0noH3V7Eh5Dc7V4vDQ9SzXjsCBZJRxTAQV/sWqscn?= =?iso-8859-1?Q?Jr3Ec3vGMRrhrzYv6+YX2mqJgh48FHXYRdCtlVdYGQyEuRudTwI5ThpAq+?= =?iso-8859-1?Q?7hYM1WrzTFBrEXZUr2jd4Q6nqPr6cbcxdumyoeyTP78pp6KVWxN1zWhmhn?= =?iso-8859-1?Q?C07kkT3nYR3VdIljcLGWbt7FF8KDNjglMlrSjpF0XwjMtSiO/EZIO8jCZq?= =?iso-8859-1?Q?z2K1FytTiMhPWN8EWzvE37PNLxhbZfXCrWZfZJVa5MIm4uOI11lHi63alC?= =?iso-8859-1?Q?FqiPIOqXH4B?= X-Microsoft-Exchange-Diagnostics: 1;BN4PR07MB2130;5:IeyXpwefDubdUM7mY9VbR/B6lce+qlxitHekHFpHt48DQrrh6rb89uk7MZYhMvW/uCVnexOnV6sbsTV79N8QWv89u0OLcEJPKTSpILBWJNnOuoAhWbO4V2b5Ks6RvtVnOS3I1YO7Qi/o/EdPEZkMhk3AUK4s5diBQBcgHA7zlibQUDzImLqumFFaEnpdxNB6;24:tf0xXd/f2bt7Lbvevnchk1TTsZJP5/NdzXLNoy3O1g+YUyMgPePbYUDGnhf6ATSlJBEm45luaYqtRCjMydbrSCNE+2sq6yL28wgEqIMRvr4=;7:/kCH84hUVZ5vLCxFMuuKZpkW7bjcVqbJp5QSUIJY7k0lpjPVGhbHNC1gVX+OyA5Nv+RMv0MEj/zQJ2lmFgpX8XaSqG7pjShmjYSb2gODWQeKVtegDIM6sVeHQossCl1OSuJOjEnK7YlmEPffsQpjaHX8WsZscfCb8lgf55AmoJjSMGHn2bV/7CnCmR9cjSJy5tCv9EawELqDYGvBURdIVFP4mYngFEvUNvBRUxH0fJE= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2016 22:28:22.6844 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN4PR07MB2130 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2926 Lines: 81 On 04/20/2016 02:52 PM, Wolfram Sang wrote: > On Mon, Apr 11, 2016 at 05:28:41PM +0200, Jan Glauber wrote: >> From: David Daney >> >> cn78xx has a different interrupt architecture, so we have to manage >> the interrupts differently. > > I'd appreciate if you could explain here why interrupts use NOAUTOEN and > have to be manually en-/disabled? This is surely unusual. It is quite nice, isn't it? Since there were enable bits in both the interrupt controller *and* the I2C bus hardware, we had redundancy that could profitably be eliminated. Now with the advent of the cn78xx, the system is much better. This redundancy has been eliminated and interrupts are enabled only in the interrupt controller. Fortunately, the kernel supplies us with convenience functions for manipulating these interrupt enable bits (enable_irq() and disable_irq()). Since we don't want to irqs enabled when the driver is probed we have to set NOAUTOEN before calling request_irq(). Personally, I preferred the old way where each device had interrupt enable bits that could be controlled independently of the interrupt controller. I have been told that I should abandon my preference for that type of antiquated architecture and welcome the brave new world of the unified interrupt enable bit. Never the less, I still feel some nostalgia for the old way. The good news is that it is not very much work to add software support for the cn78xx style interrupt enabling/masking. The hooks we add can also be used to support the MSI-X interrupts in the Thunder support patch that follows. David. > >> >> Signed-off-by: David Daney >> Signed-off-by: Jan Glauber >> --- >> drivers/i2c/busses/i2c-octeon.c | 131 ++++++++++++++++++++++++++++++++++++---- >> 1 file changed, 120 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c >> index 0bb9f0b..fff1ac0 100644 >> --- a/drivers/i2c/busses/i2c-octeon.c >> +++ b/drivers/i2c/busses/i2c-octeon.c >> @@ -11,6 +11,7 @@ >> * warranty of any kind, whether express or implied. >> */ >> >> +#include >> #include >> #include >> #include >> @@ -112,11 +113,18 @@ struct octeon_i2c { >> wait_queue_head_t queue; >> struct i2c_adapter adap; >> int irq; >> + int hlc_irq; /* For cn7890 only */ >> u32 twsi_freq; >> int sys_freq; >> void __iomem *twsi_base; >> struct device *dev; >> bool hlc_enabled; >> + void (*int_en)(struct octeon_i2c *); >> + void (*int_dis)(struct octeon_i2c *); >> + void (*hlc_int_en)(struct octeon_i2c *); >> + void (*hlc_int_dis)(struct octeon_i2c *); > > I'd prefer having 'enable' and 'disable' written out, but I leave that > to you. > >> + atomic_t int_en_cnt; >> + atomic_t hlc_int_en_cnt; >> }; >>