Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Merge tag 'scmi-fixes-5.19-2' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux into arm/fixes

Arm SCMI firmware driver fixes for v5.19

Few more fixes to address:
1. Issue reported on Juno with HDLCD clock which turned out to be yet
another firmware issue. The firmware is not conformant to the spec and
we now have to workaround as this may be copied to other platforms as
well. The spec expects to return size of 3 for a range clock rate
description while the firmware returns 1. We have other ways to validate
all the 3 entries the driver reads are polpulated and we use the same
to workaround this firmware bug.

2. Optee transport not setting the correct reponse length which is similar
to the one reported earlier on Rockchip platform.

3. Drop the usage of the deprecated ida_simple_{get,remove} and migrate to the
ida_{alloc,free}

* tag 'scmi-fixes-5.19-2' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux:
firmware: arm_scmi: Remove usage of the deprecated ida_simple_xxx API
firmware: arm_scmi: Fix response size warning for OPTEE transport
firmware: arm_scmi: Relax CLOCK_DESCRIBE_RATES out-of-spec checks

Link: https://lore.kernel.org/r/20220628133315.699803-1-sudeep.holla@arm.com
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

+38 -5
+3 -3
drivers/firmware/arm_scmi/bus.c
··· 181 181 return NULL; 182 182 } 183 183 184 - id = ida_simple_get(&scmi_bus_id, 1, 0, GFP_KERNEL); 184 + id = ida_alloc_min(&scmi_bus_id, 1, GFP_KERNEL); 185 185 if (id < 0) { 186 186 kfree_const(scmi_dev->name); 187 187 kfree(scmi_dev); ··· 204 204 put_dev: 205 205 kfree_const(scmi_dev->name); 206 206 put_device(&scmi_dev->dev); 207 - ida_simple_remove(&scmi_bus_id, id); 207 + ida_free(&scmi_bus_id, id); 208 208 return NULL; 209 209 } 210 210 ··· 212 212 { 213 213 kfree_const(scmi_dev->name); 214 214 scmi_handle_put(scmi_dev->handle); 215 - ida_simple_remove(&scmi_bus_id, scmi_dev->id); 215 + ida_free(&scmi_bus_id, scmi_dev->id); 216 216 device_unregister(&scmi_dev->dev); 217 217 } 218 218
+25 -1
drivers/firmware/arm_scmi/clock.c
··· 194 194 } 195 195 196 196 struct scmi_clk_ipriv { 197 + struct device *dev; 197 198 u32 clk_id; 198 199 struct scmi_clock_info *clk; 199 200 }; ··· 223 222 st->num_remaining = NUM_REMAINING(flags); 224 223 st->num_returned = NUM_RETURNED(flags); 225 224 p->clk->rate_discrete = RATE_DISCRETE(flags); 225 + 226 + /* Warn about out of spec replies ... */ 227 + if (!p->clk->rate_discrete && 228 + (st->num_returned != 3 || st->num_remaining != 0)) { 229 + dev_warn(p->dev, 230 + "Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n", 231 + p->clk->name, st->num_returned, st->num_remaining, 232 + st->rx_len); 233 + 234 + /* 235 + * A known quirk: a triplet is returned but num_returned != 3 236 + * Check for a safe payload size and fix. 237 + */ 238 + if (st->num_returned != 3 && st->num_remaining == 0 && 239 + st->rx_len == sizeof(*r) + sizeof(__le32) * 2 * 3) { 240 + st->num_returned = 3; 241 + st->num_remaining = 0; 242 + } else { 243 + dev_err(p->dev, 244 + "Cannot fix out-of-spec reply !\n"); 245 + return -EPROTO; 246 + } 247 + } 226 248 227 249 return 0; 228 250 } ··· 279 255 280 256 *rate = RATE_TO_U64(r->rate[st->loop_idx]); 281 257 p->clk->list.num_rates++; 282 - //XXX dev_dbg(ph->dev, "Rate %llu Hz\n", *rate); 283 258 } 284 259 285 260 return ret; ··· 298 275 struct scmi_clk_ipriv cpriv = { 299 276 .clk_id = clk_id, 300 277 .clk = clk, 278 + .dev = ph->dev, 301 279 }; 302 280 303 281 iter = ph->hops->iter_response_init(ph, &ops, SCMI_MAX_NUM_RATES,
+1
drivers/firmware/arm_scmi/driver.c
··· 1223 1223 if (ret) 1224 1224 break; 1225 1225 1226 + st->rx_len = i->t->rx.len; 1226 1227 ret = iops->update_state(st, i->resp, i->priv); 1227 1228 if (ret) 1228 1229 break;
+6 -1
drivers/firmware/arm_scmi/optee.c
··· 117 117 u32 channel_id; 118 118 u32 tee_session; 119 119 u32 caps; 120 + u32 rx_len; 120 121 struct mutex mu; 121 122 struct scmi_chan_info *cinfo; 122 123 union { ··· 303 302 return -EIO; 304 303 } 305 304 305 + /* Save response size */ 306 + channel->rx_len = param[2].u.memref.size; 307 + 306 308 return 0; 307 309 } 308 310 ··· 357 353 shbuf = tee_shm_get_va(channel->tee_shm, 0); 358 354 memset(shbuf, 0, msg_size); 359 355 channel->req.msg = shbuf; 356 + channel->rx_len = msg_size; 360 357 361 358 return 0; 362 359 } ··· 513 508 struct scmi_optee_channel *channel = cinfo->transport_info; 514 509 515 510 if (channel->tee_shm) 516 - msg_fetch_response(channel->req.msg, SCMI_OPTEE_MAX_MSG_SIZE, xfer); 511 + msg_fetch_response(channel->req.msg, channel->rx_len, xfer); 517 512 else 518 513 shmem_fetch_response(channel->req.shmem, xfer); 519 514 }
+3
drivers/firmware/arm_scmi/protocols.h
··· 179 179 * @max_resources: Maximum acceptable number of items, configured by the caller 180 180 * depending on the underlying resources that it is querying. 181 181 * @loop_idx: The iterator loop index in the current multi-part reply. 182 + * @rx_len: Size in bytes of the currenly processed message; it can be used by 183 + * the user of the iterator to verify a reply size. 182 184 * @priv: Optional pointer to some additional state-related private data setup 183 185 * by the caller during the iterations. 184 186 */ ··· 190 188 unsigned int num_remaining; 191 189 unsigned int max_resources; 192 190 unsigned int loop_idx; 191 + size_t rx_len; 193 192 void *priv; 194 193 }; 195 194