From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Date: Fri, 3 Mar 2023 11:41:10 +0100 Subject: [PATCH v6 1/2] dt-bindings: i2c: aspeed: support for AST2600-i2cv2 In-Reply-To: References: <20230226031321.3126756-1-ryan_chen@aspeedtech.com> <20230226031321.3126756-2-ryan_chen@aspeedtech.com> <53090449-58c9-bc03-56df-aa8ae93c0c26@linaro.org> Message-ID: List-Id: To: linux-aspeed@lists.ozlabs.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 03/03/2023 11:16, Ryan Chen wrote: >>>>>>> aspeed,timout properites: >>>>>>> For example I2C controller as slave mode, and suddenly >> disconnected. >>>>>>> Slave state machine will keep waiting for master clock in for >>>>>>> rx/tx >>>> transmit. >>>>>>> So it need timeout setting to enable timeout unlock controller state. >>>>>>> And in another side. In Master side also need avoid suddenly slave >>>>>> miss(un-plug), Master will timeout and release the SDA/SCL. >>>>>>> >>>>>>> Do you mean add those description into ore aspeed,timout >>>>>>> properites >>>>>> description? >>>>>> >>>>>> You are describing here one particular feature you want to enable >>>>>> in the driver which looks non-scalable and more difficult to >> configure/use. >>>>>> What I was looking for is to describe the actual configuration you have >> (e.g. >>>>>> multi-master) which leads to enable or disable such feature in your >>>> hardware. >>>>>> Especially that bool value does not scale later to actual timeout >>>>>> values in time (ms)... >>>>>> >>>>>> I don't know I2C that much, but I wonder - why this should be >>>>>> specific to Aspeed I2C and no other I2C controllers implement it? >>>>>> IOW, this looks quite generic and every I2C controller should have >>>>>> it. Adding it specific to Aspeed suggests that either we miss a >>>>>> generic property or this should not be in DT at all (because no one >>>>>> else has >>>> it...). >>>>>> >>>>>> Also I wonder, why you wouldn't enable timeout always... >>>>>> >>>>>> +Cc Wolfram, >>>>>> Maybe you know whether bool "timeout" property for one controller >>>>>> makes sense? Why we do not have it for all controllers? >>>>>> >>>>> Because, i2c bus didn?t specific timeout. >>>>> But SMBus defines a clock low time-out, TIMEOUT of 35 ms. >>>>> >>>>> It have definition in SMBus specification. >>>>> http://smbus.org/specs/SMBus_3_1_20180319.pdf >>>>> You can check Page 18, Note3 that have timeout description. >>>> >>>> Then you have already property for this - "smbus"? >>> To be a property "smbus", that would be a big topic, I saw fsl i2c >>> also have this. >>> https://github.com/torvalds/linux/blob/master/Documentation/devicetree >>> /bindings/i2c/i2c-mpc.yaml#L43-L47 >>> So, I just think the "timeout" property. >> >> Yeah and this is the only place. It also differs because it allows actual >> timeout values. > Thanks, So can I still keep the property "aspeed,timeout" here? > It is the only place. No, because none of my concerns above are addressed. Best regards, Krzysztof