From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lee Jones Subject: Re: [PATCH v3 20/20] dt-bindings: pwm: sti: Update DT bindings with recent changes Date: Wed, 13 Jul 2016 10:02:46 +0100 Message-ID: <20160713090246.GA11154@dell> References: <20160608092135.21184-1-lee.jones@linaro.org> <20160608092135.21184-21-lee.jones@linaro.org> <20160608205152.GA5511@rob-hp-laptop> <20160609114107.GB1388@dell> <20160610131855.GG27142@ulmo.ba.sec> <20160610140635.GC1537@dell> <20160610145323.GO27142@ulmo.ba.sec> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160610145323.GO27142@ulmo.ba.sec> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Thierry Reding Cc: kernel@stlinux.com, linux-pwm@vger.kernel.org, patrice.chotard@st.com, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, maxime.coquelin@st.com List-Id: linux-pwm@vger.kernel.org T24gRnJpLCAxMCBKdW4gMjAxNiwgVGhpZXJyeSBSZWRpbmcgd3JvdGU6Cj4gT24gRnJpLCBKdW4g MTAsIDIwMTYgYXQgMDM6MDY6MzVQTSArMDEwMCwgTGVlIEpvbmVzIHdyb3RlOgo+ID4gT24gRnJp LCAxMCBKdW4gMjAxNiwgVGhpZXJyeSBSZWRpbmcgd3JvdGU6Cj4gPiAKPiA+ID4gPiA+ID4gVGhl IHNlY29uZCBkb2N1bWVudGF0aW9uIGFkYXB0aW9uIGVudGFpbHMgYWRkaW5nIHN1cHBvcnQgZm9y IFBXTQo+ID4gPiA+ID4gPiBjYXB0dXJlIGRldmljZXMuICBBIG5ldyBjbG9jayBpcyByZXF1aXJl ZCBhcyB3ZWxsIGFzIGFuIElSUSBsaW5lLgo+ID4gPiA+ID4gPiBXZSdyZSBhbHNvIGFkZGluZyBh IG5ldyBwcm9wZXJ0eSBzaW1pbGFyIHRvIHRoZSBvbmUgZGVzY3JpYmVkCj4gPiA+ID4gPiA+IGFi b3ZlLCBidXQgZm9yIGNhcHR1cmUgY2hhbm5lbHMuICBUeXBpY2FsbHksIHRoZXJlIHdpbGwgYmUg bGVzcwo+ID4gPiA+ID4gPiBjYXB0dXJlIGNoYW5uZWxzIHRoYW4gUFdNLW91dCwgc2luY2UgYWxs IGNoYW5uZWxzIGhhdmUgdGhlIGxhdHRlcgo+ID4gPiA+ID4gPiBjYXBhYmlsaXR5LCBidXQgb25s eSBzb21lIGhhdmUgY2FwdHVyZSBzdXBwb3J0Lgo+ID4gPiA+ID4gCj4gPiA+ID4gPiBIdW1tLCBz b3VuZHMgbGlrZSBhbGwgb2YgdGhpcyBzaG91bGQgYmUgaW1wbGllZCBmcm9tIGNvbXBhdGlibGUg c3RyaW5ncy4KPiA+ID4gPiAKPiA+ID4gPiBZb3UgbWVhbiBoYXZlIGEgYnVuY2ggb2Ygb2ZfbWFj aGluZV9pc19jb21wYXRpYmxlcygpIHNjYXR0ZXJlZCBhcm91bmQ/Cj4gPiA+IAo+ID4gPiBJIGRv bid0IHVuZGVyc3RhbmQgd2h5IHlvdSBuZWVkIHRoaXMgYXQgYWxsLiBRdWl0ZSBmcmFua2x5IEkg ZG9uJ3QgZXZlbgo+ID4gPiBrbm93IHdoeSBzdCxwd20tbnVtLWRldnMgZXhpc3RzLiBJIHByb2Jh Ymx5IG1pc3NlZCBpdCBiYWNrIGF0IHRoZSB0aW1lLgo+ID4gPiBVc3VhbGx5LCBsaWtlIFJvYiBz dWdnZXN0cywgdGhpcyBzaG91bGQgYmUgaW5mZXJyZWQgZnJvbSB0aGUgY29tcGF0aWJsZQo+ID4g PiBzdHJpbmcuIE9uZSBjb21tb25seSB1c2VkIHdheSB0byBhdm9pZCBzY2F0dGVyaW5nIGV4cGxp Y2l0IGNoZWNrcyBmb3IKPiA+ID4gdGhlIGNvbXBhdGlibGUgc3RyaW5nIGlzIHRvIGFkZCB0aGlz IGluZm9ybWF0aW9uIHRvIHRoZSBvZl9kZXZpY2VfaWQKPiA+ID4gdGFibGUuIFNlZSBhIGJ1bmNo IG9mIGV4aXN0aW5nIGRyaXZlcnMgZm9yIHJlZmVyZW5jZS4KPiA+IAo+ID4gWWVzLCBJIGFtIGF3 YXJlIG9mIHRoZSBzdHJhdGVneSwgYW5kIGhhcHB5IHRvIG9ibGlnZSBpZiB0aGlzIGlzIHlvdXIK PiA+IHN1Z2dlc3Rpb24uICBJJ2xsIG1vdmUgYWxsIHBsYXRmb3JtIGRhdGEgaW50byB0aGUgZHJp dmVyIGFuZCBlcmFkaWNhdGUKPiA+IHRoZSBEVCBwcm9wZXJ0aWVzLgo+IAo+IEdyZWF0IQoKU29y cnkgaXQncyB0YWtlbiBzbyBsb25nIHRvIGdldCBiYWNrIGFyb3VuZCB0byB0aGlzLgoKU28gSSBq dXN0IGhhZCBhIGNyYWNrIGF0IGltcGxlbWVudGluZyB0aGlzIHNvbHV0aW9uIGFuZCByYW4gaW50 bwppc3N1ZXMuICBVbmZvcnR1bmF0ZWx5LCB0aGUgY29uZmlndXJhdGlvbiBpc24ndCBlYXN5IHRv IGRlc2NyaWJlIHVzaW5nCmNvbXBhdGlibGUgc3RyaW5ncy4KClVubGVzcyB3ZSBhcmUgcHJlcGFy ZWQgdG8gaGF2ZSBzdHJpbmdzIGxpa2U6CgogIHN0LHN0aS1wd20tc3RpaDQwNy1wd20wCiAgc3Qs c3RpLXB3bS1zdGloNDA3LXB3bTEKICBzdCxzdGktcHdtLXN0aWg0MTYtcHdtMAogIHN0LHN0aS1w d20tc3RpaDQxNi1wd20xCgpPciBwZXJoYXBzOgoKICBzdCxzdGktcHdtLTRvdXQtMmNwdAogIHN0 LHN0aS1wd20tMW91dC0xY3B0CgouLi4gc2luY2Ugc29tZSBvZiB0aGUgUFdNcyBoYXZlIGFuIG9k ZCBudW1iZXIgb2YgT1VUIGFuZCBDUFQKY2hhbm5lbHMvZGV2aWNlcy4gIEluIG15IG9waW5pb24g aG93ZXZlciwgdGhpcyBpcyB1Z2x5LiAgSSBkb24ndCB0aGluawp3ZSBkbyB0aGlzIGZvciBhbnkg b3RoZXIgdHlwZSBvZiBjaGFubmVsL2RldmljZS9wb3J0IGV0YywgZG8gd2U/CgpQZXJzb25hbGx5 IEkgdGhpbmsgdGhlcmUgaXNuJ3QgYW55dGhpbmcgd3Jvbmcgd2l0aCBoYXZpbmcgRFQKcHJvcGVy dGllcyBkZXNjcmliaW5nIGhvdyBtYW55IGNoYW5uZWxzL2RldmljZXMgdGhlcmUgYXJlLCBidXQg SSdtCm9rYXkgd2l0aCB3aGF0ZXZlciB5b3UgYm90aCBkZWNpZGUuICBObyBza2luIG9mIG15IG5v c2UgcmVhbGx5LiA6KQoKV2hhdCBzYXkgeW91PwoKLS0gCkxlZSBKb25lcwpMaW5hcm8gU1RNaWNy b2VsZWN0cm9uaWNzIExhbmRpbmcgVGVhbSBMZWFkCkxpbmFyby5vcmcg4pSCIE9wZW4gc291cmNl IHNvZnR3YXJlIGZvciBBUk0gU29DcwpGb2xsb3cgTGluYXJvOiBGYWNlYm9vayB8IFR3aXR0ZXIg fCBCbG9nCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwps aW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJh ZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51 eC1hcm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: lee.jones@linaro.org (Lee Jones) Date: Wed, 13 Jul 2016 10:02:46 +0100 Subject: [PATCH v3 20/20] dt-bindings: pwm: sti: Update DT bindings with recent changes In-Reply-To: <20160610145323.GO27142@ulmo.ba.sec> References: <20160608092135.21184-1-lee.jones@linaro.org> <20160608092135.21184-21-lee.jones@linaro.org> <20160608205152.GA5511@rob-hp-laptop> <20160609114107.GB1388@dell> <20160610131855.GG27142@ulmo.ba.sec> <20160610140635.GC1537@dell> <20160610145323.GO27142@ulmo.ba.sec> Message-ID: <20160713090246.GA11154@dell> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 10 Jun 2016, Thierry Reding wrote: > On Fri, Jun 10, 2016 at 03:06:35PM +0100, Lee Jones wrote: > > On Fri, 10 Jun 2016, Thierry Reding wrote: > > > > > > > > The second documentation adaption entails adding support for PWM > > > > > > capture devices. A new clock is required as well as an IRQ line. > > > > > > We're also adding a new property similar to the one described > > > > > > above, but for capture channels. Typically, there will be less > > > > > > capture channels than PWM-out, since all channels have the latter > > > > > > capability, but only some have capture support. > > > > > > > > > > Humm, sounds like all of this should be implied from compatible strings. > > > > > > > > You mean have a bunch of of_machine_is_compatibles() scattered around? > > > > > > I don't understand why you need this at all. Quite frankly I don't even > > > know why st,pwm-num-devs exists. I probably missed it back at the time. > > > Usually, like Rob suggests, this should be inferred from the compatible > > > string. One commonly used way to avoid scattering explicit checks for > > > the compatible string is to add this information to the of_device_id > > > table. See a bunch of existing drivers for reference. > > > > Yes, I am aware of the strategy, and happy to oblige if this is your > > suggestion. I'll move all platform data into the driver and eradicate > > the DT properties. > > Great! Sorry it's taken so long to get back around to this. So I just had a crack at implementing this solution and ran into issues. Unfortunately, the configuration isn't easy to describe using compatible strings. Unless we are prepared to have strings like: st,sti-pwm-stih407-pwm0 st,sti-pwm-stih407-pwm1 st,sti-pwm-stih416-pwm0 st,sti-pwm-stih416-pwm1 Or perhaps: st,sti-pwm-4out-2cpt st,sti-pwm-1out-1cpt ... since some of the PWMs have an odd number of OUT and CPT channels/devices. In my opinion however, this is ugly. I don't think we do this for any other type of channel/device/port etc, do we? Personally I think there isn't anything wrong with having DT properties describing how many channels/devices there are, but I'm okay with whatever you both decide. No skin of my nose really. :) What say you? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752791AbcGMJCA (ORCPT ); Wed, 13 Jul 2016 05:02:00 -0400 Received: from mail-lf0-f53.google.com ([209.85.215.53]:35899 "EHLO mail-lf0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750916AbcGMJBt (ORCPT ); Wed, 13 Jul 2016 05:01:49 -0400 Date: Wed, 13 Jul 2016 10:02:46 +0100 From: Lee Jones To: Thierry Reding Cc: Rob Herring , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@stlinux.com, maxime.coquelin@st.com, patrice.chotard@st.com, linux-pwm@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v3 20/20] dt-bindings: pwm: sti: Update DT bindings with recent changes Message-ID: <20160713090246.GA11154@dell> References: <20160608092135.21184-1-lee.jones@linaro.org> <20160608092135.21184-21-lee.jones@linaro.org> <20160608205152.GA5511@rob-hp-laptop> <20160609114107.GB1388@dell> <20160610131855.GG27142@ulmo.ba.sec> <20160610140635.GC1537@dell> <20160610145323.GO27142@ulmo.ba.sec> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160610145323.GO27142@ulmo.ba.sec> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Jun 2016, Thierry Reding wrote: > On Fri, Jun 10, 2016 at 03:06:35PM +0100, Lee Jones wrote: > > On Fri, 10 Jun 2016, Thierry Reding wrote: > > > > > > > > The second documentation adaption entails adding support for PWM > > > > > > capture devices. A new clock is required as well as an IRQ line. > > > > > > We're also adding a new property similar to the one described > > > > > > above, but for capture channels. Typically, there will be less > > > > > > capture channels than PWM-out, since all channels have the latter > > > > > > capability, but only some have capture support. > > > > > > > > > > Humm, sounds like all of this should be implied from compatible strings. > > > > > > > > You mean have a bunch of of_machine_is_compatibles() scattered around? > > > > > > I don't understand why you need this at all. Quite frankly I don't even > > > know why st,pwm-num-devs exists. I probably missed it back at the time. > > > Usually, like Rob suggests, this should be inferred from the compatible > > > string. One commonly used way to avoid scattering explicit checks for > > > the compatible string is to add this information to the of_device_id > > > table. See a bunch of existing drivers for reference. > > > > Yes, I am aware of the strategy, and happy to oblige if this is your > > suggestion. I'll move all platform data into the driver and eradicate > > the DT properties. > > Great! Sorry it's taken so long to get back around to this. So I just had a crack at implementing this solution and ran into issues. Unfortunately, the configuration isn't easy to describe using compatible strings. Unless we are prepared to have strings like: st,sti-pwm-stih407-pwm0 st,sti-pwm-stih407-pwm1 st,sti-pwm-stih416-pwm0 st,sti-pwm-stih416-pwm1 Or perhaps: st,sti-pwm-4out-2cpt st,sti-pwm-1out-1cpt ... since some of the PWMs have an odd number of OUT and CPT channels/devices. In my opinion however, this is ugly. I don't think we do this for any other type of channel/device/port etc, do we? Personally I think there isn't anything wrong with having DT properties describing how many channels/devices there are, but I'm okay with whatever you both decide. No skin of my nose really. :) What say you? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog