From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 3/6] backlight/pandora: Stop using BL_CORE_DRIVER1 Date: Mon, 30 Apr 2018 13:22:07 +0300 Message-ID: <87in899epc.fsf@intel.com> References: <20180425174253.4616-1-daniel.vetter@ffwll.ch> <20180425174253.4616-3-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0AF756E1A4 for ; Mon, 30 Apr 2018 10:19:28 +0000 (UTC) In-Reply-To: <20180425174253.4616-3-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: DRI Development , LKML Cc: Jingoo Han , Daniel Vetter , Daniel Thompson , Lee Jones , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCAyNSBBcHIgMjAxOCwgRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5j aD4gd3JvdGU6Cj4gTGVha2luZyBkcml2ZXIgaW50ZXJuYWwgdHJhY2tpbmcgaW50byB0aGUgYWxy ZWFkeSBtYXNzaXZlbHkgY29uZnVzaW5nCj4gYmFja2xpZ2h0IHBvd2VyIHRyYWNraW5nIGlzIHJl YWxseSBjb25mdXNpbmcuCj4KPiBTdG9wIHRoYXQgYnkgYWxsb2NhdGluZyBhIHRpbnkgZHJpdmVy IHByaXZhdGUgZGF0YSBzdHJ1Y3R1cmUgaW5zdGVhZC4KPgo+IENjOiBMZWUgSm9uZXMgPGxlZS5q b25lc0BsaW5hcm8ub3JnPgo+IENjOiBEYW5pZWwgVGhvbXBzb24gPGRhbmllbC50aG9tcHNvbkBs aW5hcm8ub3JnPgo+IENjOiBKaW5nb28gSGFuIDxqaW5nb29oYW4xQGdtYWlsLmNvbT4KPiBBY2tl ZC1ieTogRGFuaWVsIFRob21wc29uIDxkYW5pZWwudGhvbXBzb25AbGluYXJvLm9yZz4KPiBTaWdu ZWQtb2ZmLWJ5OiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGludGVsLmNvbT4KClJldmll d2VkLWJ5OiBKYW5pIE5pa3VsYSA8amFuaS5uaWt1bGFAaW50ZWwuY29tPgoKPiAtLS0KPiB2MjoK PiAtIENvbnNpc3RlbnRseSB0cmVhdGluZyBQQU5ET1JBX1dBU19PRkYgYXMgYSBub24tYml0Zmll bGQKPiAtIERyb3AgdGhlIGtmcmVlIHRoYXQgSSBsZWZ0IGJlaGluZCBhZnRlciBzd2l0Y2hpbmcg dG8gZGV2bV9rbWFsbG9jCj4gLS0tCj4gIGRyaXZlcnMvdmlkZW8vYmFja2xpZ2h0L3BhbmRvcmFf YmwuYyB8IDI1ICsrKysrKysrKysrKysrKysrKy0tLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDE4 IGluc2VydGlvbnMoKyksIDcgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy92 aWRlby9iYWNrbGlnaHQvcGFuZG9yYV9ibC5jIGIvZHJpdmVycy92aWRlby9iYWNrbGlnaHQvcGFu ZG9yYV9ibC5jCj4gaW5kZXggYTE4NmJjNjc3YzdkLi45NjE4NzY2ZTM4NjYgMTAwNjQ0Cj4gLS0t IGEvZHJpdmVycy92aWRlby9iYWNrbGlnaHQvcGFuZG9yYV9ibC5jCj4gKysrIGIvZHJpdmVycy92 aWRlby9iYWNrbGlnaHQvcGFuZG9yYV9ibC5jCj4gQEAgLTM1LDExICszNSwxNSBAQAo+ICAjZGVm aW5lIE1BWF9WQUxVRSA2Mwo+ICAjZGVmaW5lIE1BWF9VU0VSX1ZBTFVFIChNQVhfVkFMVUUgLSBN SU5fVkFMVUUpCj4gIAo+IC0jZGVmaW5lIFBBTkRPUkFCTF9XQVNfT0ZGIEJMX0NPUkVfRFJJVkVS MQo+ICtzdHJ1Y3QgcGFuZG9yYV9wcml2YXRlIHsKPiArCXVuc2lnbmVkIG9sZF9zdGF0ZTsKPiAr I2RlZmluZSBQQU5ET1JBQkxfV0FTX09GRiAxCj4gK307Cj4gIAo+ICBzdGF0aWMgaW50IHBhbmRv cmFfYmFja2xpZ2h0X3VwZGF0ZV9zdGF0dXMoc3RydWN0IGJhY2tsaWdodF9kZXZpY2UgKmJsKQo+ ICB7Cj4gIAlpbnQgYnJpZ2h0bmVzcyA9IGJsLT5wcm9wcy5icmlnaHRuZXNzOwo+ICsJc3RydWN0 IHBhbmRvcmFfcHJpdmF0ZSAqcHJpdiA9IGJsX2dldF9kYXRhKGJsKTsKPiAgCXU4IHI7Cj4gIAo+ ICAJaWYgKGJsLT5wcm9wcy5wb3dlciAhPSBGQl9CTEFOS19VTkJMQU5LKQo+IEBAIC01Myw3ICs1 Nyw3IEBAIHN0YXRpYyBpbnQgcGFuZG9yYV9iYWNrbGlnaHRfdXBkYXRlX3N0YXR1cyhzdHJ1Y3Qg YmFja2xpZ2h0X2RldmljZSAqYmwpCj4gIAkJYnJpZ2h0bmVzcyA9IE1BWF9VU0VSX1ZBTFVFOwo+ ICAKPiAgCWlmIChicmlnaHRuZXNzID09IDApIHsKPiAtCQlpZiAoYmwtPnByb3BzLnN0YXRlICYg UEFORE9SQUJMX1dBU19PRkYpCj4gKwkJaWYgKHByaXYtPm9sZF9zdGF0ZSA9PSBQQU5ET1JBQkxf V0FTX09GRikKPiAgCQkJZ290byBkb25lOwo+ICAKPiAgCQkvKiBmaXJzdCBkaXNhYmxlIFBXTTAg b3V0cHV0LCB0aGVuIGNsb2NrICovCj4gQEAgLTY2LDcgKzcwLDcgQEAgc3RhdGljIGludCBwYW5k b3JhX2JhY2tsaWdodF91cGRhdGVfc3RhdHVzKHN0cnVjdCBiYWNrbGlnaHRfZGV2aWNlICpibCkK PiAgCQlnb3RvIGRvbmU7Cj4gIAl9Cj4gIAo+IC0JaWYgKGJsLT5wcm9wcy5zdGF0ZSAmIFBBTkRP UkFCTF9XQVNfT0ZGKSB7Cj4gKwlpZiAocHJpdi0+b2xkX3N0YXRlID09IFBBTkRPUkFCTF9XQVNf T0ZGKSB7Cj4gIAkJLyoKPiAgCQkgKiBzZXQgUFdNIGR1dHkgY3ljbGUgdG8gbWF4LiBUUFM2MTE2 MSBzZWVtcyB0byB1c2UgdGhpcwo+ICAJCSAqIHRvIGNhbGlicmF0ZSBpdCdzIFBXTSBzZW5zaXRp dml0eSB3aGVuIGl0IHN0YXJ0cy4KPiBAQCAtOTMsOSArOTcsOSBAQCBzdGF0aWMgaW50IHBhbmRv cmFfYmFja2xpZ2h0X3VwZGF0ZV9zdGF0dXMoc3RydWN0IGJhY2tsaWdodF9kZXZpY2UgKmJsKQo+ ICAKPiAgZG9uZToKPiAgCWlmIChicmlnaHRuZXNzICE9IDApCj4gLQkJYmwtPnByb3BzLnN0YXRl ICY9IH5QQU5ET1JBQkxfV0FTX09GRjsKPiArCQlwcml2LT5vbGRfc3RhdGUgPSAwOwo+ICAJZWxz ZQo+IC0JCWJsLT5wcm9wcy5zdGF0ZSB8PSBQQU5ET1JBQkxfV0FTX09GRjsKPiArCQlwcml2LT5v bGRfc3RhdGUgPSBQQU5ET1JBQkxfV0FTX09GRjsKPiAgCj4gIAlyZXR1cm4gMDsKPiAgfQo+IEBA IC0xMDksMTMgKzExMywyMCBAQCBzdGF0aWMgaW50IHBhbmRvcmFfYmFja2xpZ2h0X3Byb2JlKHN0 cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4gIHsKPiAgCXN0cnVjdCBiYWNrbGlnaHRfcHJv cGVydGllcyBwcm9wczsKPiAgCXN0cnVjdCBiYWNrbGlnaHRfZGV2aWNlICpibDsKPiArCXN0cnVj dCBwYW5kb3JhX3ByaXZhdGUgKnByaXY7Cj4gIAl1OCByOwo+ICAKPiArCXByaXYgPSBkZXZtX2tt YWxsb2MoJnBkZXYtPmRldiwgc2l6ZW9mKCpwcml2KSwgR0ZQX0tFUk5FTCk7Cj4gKwlpZiAoIXBy aXYpIHsKPiArCQlkZXZfZXJyKCZwZGV2LT5kZXYsICJmYWlsZWQgdG8gYWxsb2NhdGUgZHJpdmVy IHByaXZhdGUgZGF0YVxuIik7Cj4gKwkJcmV0dXJuIC1FTk9NRU07Cj4gKwl9Cj4gKwo+ICAJbWVt c2V0KCZwcm9wcywgMCwgc2l6ZW9mKHByb3BzKSk7Cj4gIAlwcm9wcy5tYXhfYnJpZ2h0bmVzcyA9 IE1BWF9VU0VSX1ZBTFVFOwo+ICAJcHJvcHMudHlwZSA9IEJBQ0tMSUdIVF9SQVc7Cj4gIAlibCA9 IGRldm1fYmFja2xpZ2h0X2RldmljZV9yZWdpc3RlcigmcGRldi0+ZGV2LCBwZGV2LT5uYW1lLCAm cGRldi0+ZGV2LAo+IC0JCQkJCU5VTEwsICZwYW5kb3JhX2JhY2tsaWdodF9vcHMsICZwcm9wcyk7 Cj4gKwkJCQkJcHJpdiwgJnBhbmRvcmFfYmFja2xpZ2h0X29wcywgJnByb3BzKTsKPiAgCWlmIChJ U19FUlIoYmwpKSB7Cj4gIAkJZGV2X2VycigmcGRldi0+ZGV2LCAiZmFpbGVkIHRvIHJlZ2lzdGVy IGJhY2tsaWdodFxuIik7Cj4gIAkJcmV0dXJuIFBUUl9FUlIoYmwpOwo+IEBAIC0xMjYsNyArMTM3 LDcgQEAgc3RhdGljIGludCBwYW5kb3JhX2JhY2tsaWdodF9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1f ZGV2aWNlICpwZGV2KQo+ICAJLyogNjQgY3ljbGUgcGVyaW9kLCBPTiBwb3NpdGlvbiAwICovCj4g IAl0d2xfaTJjX3dyaXRlX3U4KFRXTF9NT0RVTEVfUFdNLCAweDgwLCBUV0xfUFdNMF9PTik7Cj4g IAo+IC0JYmwtPnByb3BzLnN0YXRlIHw9IFBBTkRPUkFCTF9XQVNfT0ZGOwo+ICsJcHJpdi0+b2xk X3N0YXRlID0gUEFORE9SQUJMX1dBU19PRkY7Cj4gIAlibC0+cHJvcHMuYnJpZ2h0bmVzcyA9IE1B WF9VU0VSX1ZBTFVFOwo+ICAJYmFja2xpZ2h0X3VwZGF0ZV9zdGF0dXMoYmwpOwoKLS0gCkphbmkg TmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5IENlbnRlcgpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0 CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752908AbeD3KT3 (ORCPT ); Mon, 30 Apr 2018 06:19:29 -0400 Received: from mga01.intel.com ([192.55.52.88]:49617 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752545AbeD3KT2 (ORCPT ); Mon, 30 Apr 2018 06:19:28 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,346,1520924400"; d="scan'208";a="51101871" From: Jani Nikula To: Daniel Vetter , DRI Development , LKML Cc: Daniel Vetter , Daniel Vetter , Daniel Thompson , Lee Jones , Jingoo Han Subject: Re: [PATCH 3/6] backlight/pandora: Stop using BL_CORE_DRIVER1 In-Reply-To: <20180425174253.4616-3-daniel.vetter@ffwll.ch> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20180425174253.4616-1-daniel.vetter@ffwll.ch> <20180425174253.4616-3-daniel.vetter@ffwll.ch> Date: Mon, 30 Apr 2018 13:22:07 +0300 Message-ID: <87in899epc.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 25 Apr 2018, Daniel Vetter wrote: > Leaking driver internal tracking into the already massively confusing > backlight power tracking is really confusing. > > Stop that by allocating a tiny driver private data structure instead. > > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > Acked-by: Daniel Thompson > Signed-off-by: Daniel Vetter Reviewed-by: Jani Nikula > --- > v2: > - Consistently treating PANDORA_WAS_OFF as a non-bitfield > - Drop the kfree that I left behind after switching to devm_kmalloc > --- > drivers/video/backlight/pandora_bl.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/video/backlight/pandora_bl.c b/drivers/video/backlight/pandora_bl.c > index a186bc677c7d..9618766e3866 100644 > --- a/drivers/video/backlight/pandora_bl.c > +++ b/drivers/video/backlight/pandora_bl.c > @@ -35,11 +35,15 @@ > #define MAX_VALUE 63 > #define MAX_USER_VALUE (MAX_VALUE - MIN_VALUE) > > -#define PANDORABL_WAS_OFF BL_CORE_DRIVER1 > +struct pandora_private { > + unsigned old_state; > +#define PANDORABL_WAS_OFF 1 > +}; > > static int pandora_backlight_update_status(struct backlight_device *bl) > { > int brightness = bl->props.brightness; > + struct pandora_private *priv = bl_get_data(bl); > u8 r; > > if (bl->props.power != FB_BLANK_UNBLANK) > @@ -53,7 +57,7 @@ static int pandora_backlight_update_status(struct backlight_device *bl) > brightness = MAX_USER_VALUE; > > if (brightness == 0) { > - if (bl->props.state & PANDORABL_WAS_OFF) > + if (priv->old_state == PANDORABL_WAS_OFF) > goto done; > > /* first disable PWM0 output, then clock */ > @@ -66,7 +70,7 @@ static int pandora_backlight_update_status(struct backlight_device *bl) > goto done; > } > > - if (bl->props.state & PANDORABL_WAS_OFF) { > + if (priv->old_state == PANDORABL_WAS_OFF) { > /* > * set PWM duty cycle to max. TPS61161 seems to use this > * to calibrate it's PWM sensitivity when it starts. > @@ -93,9 +97,9 @@ static int pandora_backlight_update_status(struct backlight_device *bl) > > done: > if (brightness != 0) > - bl->props.state &= ~PANDORABL_WAS_OFF; > + priv->old_state = 0; > else > - bl->props.state |= PANDORABL_WAS_OFF; > + priv->old_state = PANDORABL_WAS_OFF; > > return 0; > } > @@ -109,13 +113,20 @@ static int pandora_backlight_probe(struct platform_device *pdev) > { > struct backlight_properties props; > struct backlight_device *bl; > + struct pandora_private *priv; > u8 r; > > + priv = devm_kmalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + dev_err(&pdev->dev, "failed to allocate driver private data\n"); > + return -ENOMEM; > + } > + > memset(&props, 0, sizeof(props)); > props.max_brightness = MAX_USER_VALUE; > props.type = BACKLIGHT_RAW; > bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev, > - NULL, &pandora_backlight_ops, &props); > + priv, &pandora_backlight_ops, &props); > if (IS_ERR(bl)) { > dev_err(&pdev->dev, "failed to register backlight\n"); > return PTR_ERR(bl); > @@ -126,7 +137,7 @@ static int pandora_backlight_probe(struct platform_device *pdev) > /* 64 cycle period, ON position 0 */ > twl_i2c_write_u8(TWL_MODULE_PWM, 0x80, TWL_PWM0_ON); > > - bl->props.state |= PANDORABL_WAS_OFF; > + priv->old_state = PANDORABL_WAS_OFF; > bl->props.brightness = MAX_USER_VALUE; > backlight_update_status(bl); -- Jani Nikula, Intel Open Source Technology Center