From owner-freebsd-hackers@freebsd.org Wed May 29 06:57:05 2019 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 62AFB15B971B for ; Wed, 29 May 2019 06:57:05 +0000 (UTC) (envelope-from agapon@gmail.com) Received: from mail-lj1-f173.google.com (mail-lj1-f173.google.com [209.85.208.173]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 496B184B56; Wed, 29 May 2019 06:57:04 +0000 (UTC) (envelope-from agapon@gmail.com) Received: by mail-lj1-f173.google.com with SMTP id m22so1081047ljc.3; Tue, 28 May 2019 23:57:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=HCKRu6897mgPCMH6Obja36q9U2ja1oozgvxFMDaGVhc=; b=TJ9rQ5ww+ZMG/1h88pgBGHS+X8tJpEsCGp2T+r4/0O/n5ryAARLxJcCwhsSH7vHCc0 Rp156/toO0zprJbkUFgdxlr1Jcgeotk2NXn6VJXP4puy6fgyD0QZ7sHqWL9B4YIJQjuI TGvP65/NkbY6rvfDrQJgJuakHNA+1dCMa/RhR2vrrgQbbdMjIhgkPEPahqbOdgxhcPyQ SrW/SRZnGr+eZzoIGVA9RM+efJ4ek7TSrow5gCmuCH/LtTcfgR/TMLiZFN6xzhqel+oX 3GD10yPkm5Z+Bligt21405HKV/LnYxwgTP+ng5n74jo4uJmjOJLsCYDbS4YrUKIW5y0X nApQ== X-Gm-Message-State: APjAAAWAZoSfNqvvjNzPwlXUAEmR1kObxYUmfJzzgd2gQqH+7povsVcc qkIBxyGNUUuD3iz8UQijylnodhlA X-Google-Smtp-Source: APXvYqy7O7VidWJpOJwOI80iWhACHRAIfOdS5+DSEazt4CNjJmYEgVLgcstEaSvuB2rn51jmgDVf+Q== X-Received: by 2002:a2e:860a:: with SMTP id a10mr10681736lji.158.1559113022283; Tue, 28 May 2019 23:57:02 -0700 (PDT) Received: from [192.168.0.88] (east.meadow.volia.net. [93.72.151.96]) by smtp.googlemail.com with ESMTPSA id z24sm3299871lfh.63.2019.05.28.23.57.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 May 2019 23:57:01 -0700 (PDT) Subject: Re: completely broken IICBUS_IVAR_NOSTOP From: Andriy Gapon To: Ian Lepore , FreeBSD Hackers , Konstantin Belousov References: <1521383420.99081.87.camel@freebsd.org> <0b0dee4b-e958-e25c-72d9-1ca296495802@FreeBSD.org> Openpgp: preference=signencrypt Autocrypt: addr=avg@FreeBSD.org; prefer-encrypt=mutual; keydata= xsFNBFm4LIgBEADNB/3lT7f15UKeQ52xCFQx/GqHkSxEdVyLFZTmY3KyNPQGBtyvVyBfprJ7 mAeXZWfhat6cKNRAGZcL5EmewdQuUfQfBdYmKjbw3a9GFDsDNuhDA2QwFt8BmkiVMRYyvI7l N0eVzszWCUgdc3qqM6qqcgBaqsVmJluwpvwp4ZBXmch5BgDDDb1MPO8AZ2QZfIQmplkj8Y6Z AiNMknkmgaekIINSJX8IzRzKD5WwMsin70psE8dpL/iBsA2cpJGzWMObVTtCxeDKlBCNqM1i gTXta1ukdUT7JgLEFZk9ceYQQMJJtUwzWu1UHfZn0Fs29HTqawfWPSZVbulbrnu5q55R4PlQ /xURkWQUTyDpqUvb4JK371zhepXiXDwrrpnyyZABm3SFLkk2bHlheeKU6Yql4pcmSVym1AS4 dV8y0oHAfdlSCF6tpOPf2+K9nW1CFA8b/tw4oJBTtfZ1kxXOMdyZU5fiG7xb1qDgpQKgHUX8 7Rd2T1UVLVeuhYlXNw2F+a2ucY+cMoqz3LtpksUiBppJhw099gEXehcN2JbUZ2TueJdt1FdS ztnZmsHUXLxrRBtGwqnFL7GSd6snpGIKuuL305iaOGODbb9c7ne1JqBbkw1wh8ci6vvwGlzx rexzimRaBzJxlkjNfMx8WpCvYebGMydNoeEtkWldtjTNVsUAtQARAQABzR5BbmRyaXkgR2Fw b24gPGF2Z0BGcmVlQlNELm9yZz7CwZQEEwEIAD4WIQS+LEO7ngQnXA4Bjr538m7TUc1yjwUC WbgsiAIbIwUJBaOagAULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAKCRB38m7TUc1yj+JAEACV l9AK/nOWAt/9cufV2fRj0hdOqB1aCshtSrwHk/exXsDa4/FkmegxXQGY+3GWX3deIyesbVRL rYdtdK0dqJyT1SBqXK1h3/at9rxr9GQA6KWOxTjUFURsU7ok/6SIlm8uLRPNKO+yq0GDjgaO LzN+xykuBA0FlhQAXJnpZLcVfPJdWv7sSHGedL5ln8P8rxR+XnmsA5TUaaPcbhTB+mG+iKFj GghASDSfGqLWFPBlX/fpXikBDZ1gvOr8nyMY9nXhgfXpq3B6QCRYKPy58ChrZ5weeJZ29b7/ QdEO8NFNWHjSD9meiLdWQaqo9Y7uUxN3wySc/YUZxtS0bhAd8zJdNPsJYG8sXgKjeBQMVGuT eCAJFEYJqbwWvIXMfVWop4+O4xB+z2YE3jAbG/9tB/GSnQdVSj3G8MS80iLS58frnt+RSEw/ psahrfh0dh6SFHttE049xYiC+cM8J27Aaf0i9RflyITq57NuJm+AHJoU9SQUkIF0nc6lfA+o JRiyRlHZHKoRQkIg4aiKaZSWjQYRl5Txl0IZUP1dSWMX4s3XTMurC/pnja45dge/4ESOtJ9R 8XuIWg45Oq6MeIWdjKddGhRj3OohsltKgkEU3eLKYtB6qRTQypHHUawCXz88uYt5e3w4V16H lCpSTZV/EVHnNe45FVBlvK7k7HFfDDkryM7BTQRZuCyIARAAlq0slcsVboY/+IUJdcbEiJRW be9HKVz4SUchq0z9MZPX/0dcnvz/gkyYA+OuM78dNS7Mbby5dTvOqfpLJfCuhaNYOhlE0wY+ 1T6Tf1f4c/uA3U/YiadukQ3+6TJuYGAdRZD5EqYFIkreARTVWg87N9g0fT9BEqLw9lJtEGDY EWUE7L++B8o4uu3LQFEYxcrb4K/WKmgtmFcm77s0IKDrfcX4doV92QTIpLiRxcOmCC/OCYuO jB1oaaqXQzZrCutXRK0L5XN1Y1PYjIrEzHMIXmCDlLYnpFkK+itlXwlE2ZQxkfMruCWdQXye syl2fynAe8hvp7Mms9qU2r2K9EcJiR5N1t1C2/kTKNUhcRv7Yd/vwusK7BqJbhlng5ZgRx0m WxdntU/JLEntz3QBsBsWM9Y9wf2V4tLv6/DuDBta781RsCB/UrU2zNuOEkSixlUiHxw1dccI 6CVlaWkkJBxmHX22GdDFrcjvwMNIbbyfQLuBq6IOh8nvu9vuItup7qemDG3Ms6TVwA7BD3j+ 3fGprtyW8Fd/RR2bW2+LWkMrqHffAr6Y6V3h5kd2G9Q8ZWpEJk+LG6Mk3fhZhmCnHhDu6CwN MeUvxXDVO+fqc3JjFm5OxhmfVeJKrbCEUJyM8ESWLoNHLqjywdZga4Q7P12g8DUQ1mRxYg/L HgZY3zfKOqcAEQEAAcLBfAQYAQgAJhYhBL4sQ7ueBCdcDgGOvnfybtNRzXKPBQJZuCyIAhsM BQkFo5qAAAoJEHfybtNRzXKPBVwQAKfFy9P7N3OsLDMB56A4Kf+ZT+d5cIx0Yiaf4n6w7m3i ImHHHk9FIetI4Xe54a2IXh4Bq5UkAGY0667eIs+Z1Ea6I2i27Sdo7DxGwq09Qnm/Y65ADvXs 3aBvokCcm7FsM1wky395m8xUos1681oV5oxgqeRI8/76qy0hD9WR65UW+HQgZRIcIjSel9vR XDaD2HLGPTTGr7u4v00UeTMs6qvPsa2PJagogrKY8RXdFtXvweQFz78NbXhluwix2Tb9ETPk LIpDrtzV73CaE2aqBG/KrboXT2C67BgFtnk7T7Y7iKq4/XvEdDWscz2wws91BOXuMMd4c/c4 OmGW9m3RBLufFrOag1q5yUS9QbFfyqL6dftJP3Zq/xe+mr7sbWbhPVCQFrH3r26mpmy841ym dwQnNcsbIGiBASBSKksOvIDYKa2Wy8htPmWFTEOPRpFXdGQ27awcjjnB42nngyCK5ukZDHi6 w0qK5DNQQCkiweevCIC6wc3p67jl1EMFY5+z+zdTPb3h7LeVnGqW0qBQl99vVFgzLxchKcl0 R/paSFgwqXCZhAKMuUHncJuynDOP7z5LirUeFI8qsBAJi1rXpQoLJTVcW72swZ42IdPiboqx NbTMiNOiE36GqMcTPfKylCbF45JNX4nF9ElM0E+Y8gi4cizJYBRr2FBJgay0b9Cp Message-ID: <4de0ed33-47bb-3452-4563-adee8c5070de@FreeBSD.org> Date: Wed, 29 May 2019 09:57:00 +0300 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 496B184B56 X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; spf=pass (mx1.freebsd.org: domain of agapon@gmail.com designates 209.85.208.173 as permitted sender) smtp.mailfrom=agapon@gmail.com X-Spamd-Result: default: False [-4.10 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; DMARC_NA(0.00)[FreeBSD.org]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; RCVD_COUNT_THREE(0.00)[3]; RCVD_TLS_LAST(0.00)[]; TO_DN_ALL(0.00)[]; MX_GOOD(-0.01)[cached: alt3.gmail-smtp-in.l.google.com]; NEURAL_HAM_SHORT(-0.83)[-0.834,0]; RCVD_IN_DNSWL_NONE(0.00)[173.208.85.209.list.dnswl.org : 127.0.5.0]; IP_SCORE(-1.26)[ip: (-0.57), ipnet: 209.85.128.0/17(-3.38), asn: 15169(-2.29), country: US(-0.06)]; FORGED_SENDER(0.30)[avg@FreeBSD.org,agapon@gmail.com]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[avg@FreeBSD.org,agapon@gmail.com]; MID_RHS_MATCH_FROM(0.00)[] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 May 2019 06:57:05 -0000 On 24/05/2019 20:43, Andriy Gapon wrote: > > A lot of time has passed but the problem is still there. > The "nostop" thing is completely broken. > I have recently thought about this issue a little bit again and now I think that > IVARs are not suitable for nostop property. Simply put, there is no bus (in the > "newbus" sense) where such an IVAR could be configured for a child. > > I see two possible solutions. > > 1. Make "nostop" a property of an iicbus instance. We can keep the current > getter and setter, but they won't be IVAR accessors any more. Also, target > devices for the calls to the functions would need to be adjusted. > > 2. Since this property hasn't found a wider use and remains specific to the > intel drm driver, we could just subclass iicbb and put the quirk into the > subclassed driver. Actually, the only consumer of this feature, drm2, has already been removed from the tree. And the out-of-tree drm driver uses linux i2c implementation in linuxkpi. So, it does not use FreeBSD i2c code at all. Thus, we can completely remove the broken code. And, if we decide to revisit the original problem later, we can design a new solution. Like making NOSTOP a default and, perhaps, adding an explicit STOP flag. > On 23/03/2018 11:56, Andriy Gapon wrote: >> On 18/03/2018 16:30, Ian Lepore wrote: >>> Now for the bad news:  don't use it.  It doesn't work.  It's 100% a bug >>> in the code that maybe kinda-sorta seemed to work for whoever added it, >>> because accidentally the right garbage was on the stack in the local >>> nostop var.  The generic transfer code doesn't check that the accessor >>> failed so it ends up using stack garbage for nostop.  The reason >>> there's g'teed to be no such ivar is because the code is asking the >>> wrong device, and it doesn't even have a handle to the right child >>> device to get the info it wants. >> >> >> Oh, indeed. >> I think that there never was an intention to make "nostop" a property of an i2c >> slave, a child of an iicbus device. I think that instead it was supposed to be >> a property of the iicbus's parent device, an actual i2c adapter driver. >> I guess that the reason that "nostop" became an ivar in iicbus was an incorrect >> understanding of how a "bit-banger" device (something implementing iicbb_if), >> iicbb device and iicbus device are connected. I think that I was among the >> reviewers and I probably had a bit of confusion back then. >> >> >> It seems that the only place where iicbus_get_nostop() is used is >> iicbus_transfer_gen(). iicbus_transfer_gen is used in several i2c drivers as an >> iicbus_transfer method. it's also used in iicbb, thinly wrapped by >> iicbb_transfer(). >> So, iicbus_get_nostop() actually translates to a call to BUS_READ_IVAR(parent, >> device, 1, &v) where I already substituted one for IICBUS_IVAR_NOSTOP. >> Here we can see that while the accessor functions lok quite nice they get >> expanded to generic calls without much context. So, whether that >> BUS_READ_IVAR() call succeeds and what value it gets depends on whether the >> parent bus defines an ivar with a value of 1 and what value that ivar might have >> for the driver device. Many buses define at least a couple of ivars. >> So, a return value of iicbus_get_nostop() could be consistent for a particular >> driver while still being arbitrary. But it can be a piece of stack garbage too, >> just as you said. >> >> The only place where iicbus_set_nostop() is used is intel_iicbb_attach(). >> In that case the ivar is "set" on a wrong device at all (the bit-banger, not iicbb). >> >> >> This definitely needs to be fixed / reworked. >> Perhaps nostop should become a new interface in iicbus_if and iicbb_if... >> > > -- Andriy Gapon