Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752515AbcDVHVz (ORCPT ); Fri, 22 Apr 2016 03:21:55 -0400 Received: from 7of9.schinagl.nl ([88.159.158.68]:35182 "EHLO 7of9.schinagl.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751697AbcDVHVx (ORCPT ); Fri, 22 Apr 2016 03:21:53 -0400 Subject: Re: [PATCHv1 0/6] leds: pca9653x: support inverted outputs and cleanups To: Ricardo Ribalda Delgado References: <1461051650-18824-1-git-send-email-oliver@schinagl.nl> <5715F927.3030102@samsung.com> <5715FCE8.7080106@schinagl.nl> <57163252.5090000@schinagl.nl> <57172DFA.9030107@schinagl.nl> <5717430D.30702@schinagl.nl> <571746A2.8040609@schinagl.nl> Cc: Jacek Anaszewski , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Richard Purdie , "devicetree@vger.kernel.org" , LKML , Linux LED Subsystem , Peter Meerwald From: Olliver Schinagl Message-ID: <5719D0FF.7090800@schinagl.nl> Date: Fri, 22 Apr 2016 09:21:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2003 Lines: 57 Hi Ricardo, On 20-04-16 11:17, Ricardo Ribalda Delgado wrote: > Hello again > > On Wed, Apr 20, 2016 at 11:06 AM, Olliver Schinagl wrote: > >> The devil is in the details :) > :) >>> Saving mode2 sounds like a good compromise then. >>> >>> But I still believe that we should limit the lock to ledout. No matter >>> what we do, we cannot have two leds blinking at different frequencies >>> on the same chip. >> So to save a mutex a little bit, we take the risk that nobody else enables >> the blink or if they do, enable it in the same way? >> If it saves so much, then I guess its worth the risk I suppose? > Give me a day to go through the chip doc and see if I can find a good > compromise, that at least warranties that the leds that are enable > stay enabled ;) Right, I also went over the datasheet, and I think we can simplyfy two things. For one, yes, move the mode2 register completly to the probe section. Set the DMBLINK led to always 1. It does not get cleared, I was wrong. We have to set it to as with 0 we do not get any blinking at all (grpfreq gets ignored). Furthermore, we should change: - gdc = (time_on * 256) / period; + gdc = 0x00; Because the calculation does not make sense. GDC is the global brightness/pwm/dimming control. It is used to uniformly change the blink rate on all the linked leds. "General brightness for the 16 outputs is controlled through 256 linear steps to FFh" I don't think that is the intention of the gdc is it? Looking at the original gdc code, it thus sets the global BRIGHTNESS based on the period/on_time. I don't think that is what we expect when we enable blink. From my understanding, the grppwm is super-imposed, thus by setting gdc to 0, we do not superimpose anything and the original brightness is retained. (If i'm wrong here, we need to set gdc to 0xff. Because of this, I even recommend removing gdc all together, which saves another i2c write. Or am I wrong here? Olliver > > Regards! > > >