Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <634209c7-e458-8c4c-7bef-63f283aefaf7@os.amperecomputing.com>
Date:   Sat, 15 Oct 2022 10:24:26 +0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.3.2
Subject: Re: [PATCH v10 1/3] ipmi: ssif_bmc: Add SSIF BMC driver
Content-Language: en-CA
To:     Graeme Gregory <quic_ggregory@quicinc.com>
Cc:     devicetree@vger.kernel.org, Randy Dunlap <rdunlap@infradead.org>,
        linux-aspeed@lists.ozlabs.org, Corey Minyard <minyard@acm.org>,
        Andrew Jeffery <andrew@aj.id.au>, openbmc@lists.ozlabs.org,
        thang@os.amperecomputing.com,
        Phong Vo <phong@os.amperecomputing.com>,
        linux-kernel@vger.kernel.org, Wolfram Sang <wsa@kernel.org>,
        Brendan Higgins <brendan.higgins@linux.dev>,
        Rob Herring <robh+dt@kernel.org>,
        Joel Stanley <joel@jms.id.au>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        openipmi-developer@lists.sourceforge.net,
        Open Source Submission <patches@amperecomputing.com>,
        linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org
References: <20221004093106.1653317-1-quan@os.amperecomputing.com>
 <20221004093106.1653317-2-quan@os.amperecomputing.com>
 <5fbc5a54-60f2-fc0d-a4a1-839f28a4d8ba@quicinc.com>
 <48a5e252-9a1e-65e6-e8bf-add3d39a0286@os.amperecomputing.com>
 <72c8a5bc-830d-25a9-0528-5d428dd9f163@quicinc.com>
 <20221014130517.y4mdinylnwfrg6u2@ggregory-linuxws>
From:   Quan Nguyen <quan@os.amperecomputing.com>
In-Reply-To: <20221014130517.y4mdinylnwfrg6u2@ggregory-linuxws>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NG4yTWlSUkFGOEgvZ3d4T3BJVjFhL0FMb3kvQ3h4K1p3ZEY2aXl5ckhvbkhO?=
 =?utf-8?B?UzlIcWJFUW1hNlhSZ1BNdndSbGxHTEtXNGwvbXk5WGlMNnBsd21KWWdFTmdu?=
 =?utf-8?B?QVNsWEVVYWZKVjJWdlppalFtN1FIcWVrMDhNcEliem4vRy8rWWExUmplQWFz?=
 =?utf-8?B?ZlNxREJhWjFjSHp0SFRIRmppeEJIYVZlTVFCQ0V5VmlNWjdaOTdsRGJFUVVF?=
 =?utf-8?B?WVRYNzUrc0tCWCtqaU5lUFRXZVdwWndGakQ0UFkrdXMrdUw3SWxoRUhyK2sr?=
 =?utf-8?B?NWZLVWVjRSt0NVphQUErR3hnR2I2M1BPNEFOTUFmc3R3NFFvY0Y0djJIOVVk?=
 =?utf-8?B?cFBaSGxpMXljenFLMEtBMVR0dTJmUDZqRGNlQWYvUjV4Q3RtWENla3E2VDl3?=
 =?utf-8?B?cDIwSm5sa1g3ZXlSY3B2elNTekpGVm9iUGlFcDM2VStaeURoYmJZR3F5bVlj?=
 =?utf-8?B?Q1hZY29WR05ub3lKVVQvVUZadTZCY0w3T2t3WVhScGxGeFhhWjJDdVRrcFoy?=
 =?utf-8?B?eDJTLzJQd3kzbHhHdHg0aG05ZHN4K3hBWWk0QTBWclVXL1ZnOU5nQm9uQ3Rn?=
 =?utf-8?B?VkZucno4SUZ3TWJ6clN3V3ZFdVZnYWdoejgrMmpMd3lHdXlvSDQvZUgzNys1?=
 =?utf-8?B?RHVaUThrbkRnS2ovdUU3L3NZd3pITFZickJvYWJSOTljdDJ3WmtqcVVVRTk1?=
 =?utf-8?B?SVczRk0rUFpWQ3pHTWRCeGF5WUlHbkhBNzdMcEN5bVNORE9DVkZKRTBuOXZu?=
 =?utf-8?B?QVphVXo0dEJYRWg0RnlkUXNuanh6ZmNkNng4aTNVL1BseDNGSWM4MUlhZTdu?=
 =?utf-8?B?Q1JFcU9UeTErcW9yaUVRbUo4YVpYSlJsU3h6bGMwZlBHQjFMNTgvZUU3SmMv?=
 =?utf-8?B?d3BmcmQ3T2xraDFDMEFYZkdrR1hyMVZvTVFHQTE3Z1dBZVI2ZFlSZWR1c2Va?=
 =?utf-8?B?eDJvNWIveHJpTEd3dUg3dnh0YlA1UVQyN3Zqd29TUmFrZW9WODFzTzFpT09T?=
 =?utf-8?B?VkFVY1Zoc0RscThma2Nla0ZDTXEzOGV0bjFQbWd4K05mYVJyVTU2Z3RpVExT?=
 =?utf-8?B?TzR1SWcvajJWWXZ1SWVGeHVvOUkxMm4yaDJVb3dBOU1WaDVSMXA2WUZ5T2tG?=
 =?utf-8?B?ZSs0S3BNUTQwWUc5b0V0em1tT3Znc3FLeWJFYkFiVmNsMlhrQUVWb1ZGQjk0?=
 =?utf-8?B?OGh1SGVRZGZXRExGSk1tZGRTeHhMQjNtZGJuaWpBZG5JZFMyVzFNZkxDVVhZ?=
 =?utf-8?B?VzNxMnNUTS94b3cvYXFEclJYVDlrUlJoU2hxTVFoallSczgxMHNkblpacmFw?=
 =?utf-8?B?OGlmdnU5ZzVsOEwxUTBQeGNXSjJhcVdmV28yUTUvdzB5YnkwNXFHSXJWYVRK?=
 =?utf-8?B?djNyUGRreXJ2Wm1SYWlGdFl3dzl3YnJKVFZmNEJtUDZnbGhxT1RLaVcvblgv?=
 =?utf-8?B?dXJMd2JkMmQxSm9HT1Qrci82SjRvaDlacmZ2ZkdabTJaK1hVVWlrNFB6NStE?=
 =?utf-8?B?SlpEOCt2S3FNdkd0eDgzVXhUTFZXYlBoSVg1R0RSUlRYZGY4akRHLy9tQnUv?=
 =?utf-8?B?aXBPSUc0Q2FxL0Mvd0UreFg5WHFmL2o0WEhrTitJcTZNdWtJbG10RmQ0TWZt?=
 =?utf-8?B?SHdxcm5XS0VzRVBtVFF2eGJoVFlBV3lnbWVESnZWWW1UV3VITmh1RDd5TStv?=
 =?utf-8?B?dTZrYXRtMTBUOUJRMHM0V2I2UkFOZytkbTB5bWd2VXU5Tmt3aGxYSzk3K21L?=
 =?utf-8?B?S1VOWG04YU5vSWtDajhmZSt6Nm1rMk9qOW4rNThBQkJ5cDZkSURBQW5ZK0Ex?=
 =?utf-8?B?blF3NXdzVDdWNXJIYjdCTEpGREtMUDFwYUlJNzUycjdyZVpiTXZ0QUpKME10?=
 =?utf-8?B?VGhrYWJjR3V6K0J6ak02UkNvODhjb2NJVDFOWmRxSmxxdS9TVCt0ejJld25W?=
 =?utf-8?B?ODJibmNpeUJDaVNJQTBDSUJadEwvcGNGQjNQYVBkaUZSaDdvZGlReDBIQ0dl?=
 =?utf-8?B?WWhKMGRHeFdiWGdlaUxxbGhhRUM1WkpHTFRXMnVXSVM5R2Y1b0FvVlFsMmVh?=
 =?utf-8?B?UWxJUHhSOUc1cXpmenRxb25jSU1tbWg1dGJIU2xtTkpkK1FNMHZDYi90M0xK?=
 =?utf-8?B?R05HZUlPZk5LcWl3STJ2ODRDUytUZ3JnejN3cWFZVnNCN1ptMHpoaVlDd2NS?=
 =?utf-8?Q?1MSmzHovlAD4tauoE0IeoWY=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 441b2cca-33ff-45c2-841c-08daae5cd1c4
X-MS-Exchange-CrossTenant-AuthSource: SN4PR01MB7455.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Oct 2022 03:24:51.2136
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ZTS+nIy73ubGXKHK4rqAWISFmqJY00hBTlw55/jDDS8Ga8pdIobJS21rR7R7Nh8SWQOk/it67i2eeLp3S/BWvGtjBeD2F27ou0VfE0ogOIw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR01MB4547
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 14/10/2022 20:05, Graeme Gregory wrote:
> On Mon, Oct 10, 2022 at 12:08:24PM +0100, Graeme Gregory wrote:
>>
>> On 10/10/2022 02:28, Quan Nguyen wrote:
>>>
>>>
>>> On 07/10/2022 20:26, Graeme Gregory wrote:
>>>>
>>>> On 04/10/2022 10:31, Quan Nguyen wrote:
>>>>> The SMBus system interface (SSIF) IPMI BMC driver can be used to
>>>>> perform
>>>>> in-band IPMI communication with their host in management (BMC) side.
>>>>>
>>>>> Thanks Dan for the copy_from_user() fix in the link below.
>>>>>
>>>>> Link: https://lore.kernel.org/linux-arm-kernel/20220310114119.13736-4-quan@os.amperecomputing.com/
>>>>> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com>
>>>>
>>>> I have been testing this on our hardware and I don't seem to be able
>>>> to get this driver working. I was using an older version.
>>>>
>>>> I have hacked ssifbridged to deal with the change in len from u8 to
>>>> unsigned int.
>>>>
>>>> It works as long as I only ever send SSIF commands, any attempt to
>>>> read a response crashes the state machine and the driver never
>>>> recovers. No further SSIF comms is possible! (slave doesnt even ACK
>>>> writes).
>>>>
>>>> A couple of comments below on possible state machine errors.
>>>>
>>>> Its possible I am doing something wrong!
>>>>
>>>
>>> Thanks Graeme for the test and the comments.
>>>
>>> What's your testing hardware?
>>>
>>> This was tested with Aspeed ast2500 and ast2600 with the patch series
>>> [1] below applied.
>>>
>>> If you use the same hw, could you pick the series and see if any thing
>>> improve ?
>>>
>>> [1] https://lore.kernel.org/all/20210616031046.2317-1-quan@os.amperecomputing.com/
>>>
>> Thanks, that patch series does stop the state machine crashing.
>>
>> I am testing on AST2600EVB with A1 rev, but we also have our own DC-SCM with
>> A3 chip.
>>
>> Responses are still not working for me, but I think that may be an error in
>> my ssifbridged hacks.
>>
>> Oct 10 10:54:55 qcom-evb-proto-ccf37d18ea0c ssifbridged[335]: Read ssif
>> request message with len=13 netfn=44 lun=0 cmd=2
>>
>> Oct 10 10:54:55 qcom-evb-proto-ccf37d18ea0c ipmid[312]: BootCode:
>> 000000000000000000
>>
>> Oct 10 10:54:55 qcom-evb-proto-ccf37d18ea0c ssifbridged[335]: Send ssif
>> respond message with len=4 netfn=45 lun=0 cmd=2 cc=0
>>
>> Oct 10 10:54:55 qcom-evb-proto-ccf37d18ea0c kernel: ipmi-ssif-host 0-0010:
>> Warn: on_read_requested_event unexpected READ REQUESTED in state=SSIF_READY
>>
>> Oct 10 10:54:55 qcom-evb-proto-ccf37d18ea0c kernel: ipmi-ssif-host 0-0010:
>> Warn: on_stop_event unexpected SLAVE STOP in state=SSIF_ABORTING
>>
> Just to close the loop on this, I have now fully tested this driver on
> our setup internally.
> 
> Final hitch turned out to be an aardvark not issuing STOP events between
> reads/writes.
> 
> Thanks for the work.
> 

Thanks a lot Graeme, for the test.

- Quan