On 15/03/2022 01:15, Jae Hyun Yoo wrote:
Hi Quan,
[...]
+static void on_read_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8
*val)
+{
+ if (ssif_bmc->state == SSIF_READY ||
+ ssif_bmc->state == SSIF_START ||
+ ssif_bmc->state == SSIF_REQ_RECVING ||
+ ssif_bmc->state == SSIF_RES_SENDING) {
+ ssif_bmc->state = SSIF_BAD_SMBUS;
+ dev_warn(&ssif_bmc->client->dev,
+ "Warn: %s unexpected READ REQUESTED in state=%s\n",
+ __func__, state_to_string(ssif_bmc->state));
It will print out that the case happens in SSIF_BAD_SMBUS state always
because ssif_bmc->state is already assigned as SSIF_BAD_SMBUS above.
Move the assignment to after the dev_warn printing.
[...]
Thanks for pointing this out. Will fix this in next version.
+static const struct of_device_id ssif_bmc_match[] = {
+ { .compatible = "ampere,ssif-bmc" },
Does this driver have any Ampere specific handling? If not, it could be
"ssif-bmc" as a generic SSIF driver.
Will change as you suggested in next version.
+ { },
+};
Add 'MODULE_DEVICE_TABLE(of, ssif_bmc_match);'
Will do in next version
+
+static const struct i2c_device_id ssif_bmc_id[] = {
+ { DEVICE_NAME, 0 },
+ { },
+};
+
Remove this blank line.
Will update in next version
+MODULE_DEVICE_TABLE(i2c, ssif_bmc_id);
+
+static struct i2c_driver ssif_bmc_driver = {
+ .driver = {
+ .name = DEVICE_NAME,
+ .of_match_table = ssif_bmc_match,
+ },
+ .probe = ssif_bmc_probe,
+ .remove = ssif_bmc_remove,
+ .id_table = ssif_bmc_id,
+};
+
Remove this blank line too.
-Jae
Thanks Jae for the review, will update in next version.
- Quan
+module_i2c_driver(ssif_bmc_driver);
+
+MODULE_AUTHOR("Quan Nguyen <quan@xxxxxxxxxxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Chuong Tran <chuong@xxxxxxxxxxxxxxxxxxxxxx>");
+MODULE_DESCRIPTION("Linux device driver of the BMC IPMI SSIF
interface.");
+MODULE_LICENSE("GPL");
[...]
