On 2/24/2023 8:25 AM, Stanislaw Gruszka wrote:
On Mon, Feb 06, 2023 at 08:41:42AM -0700, Jeffrey Hugo wrote:
+#define SEM_VAL_MASK GENMASK_ULL(11, 0)
+#define SEM_INDEX_MASK GENMASK_ULL(4, 0)
+#define BULK_XFER BIT(3)
+#define GEN_COMPLETION BIT(4)
+#define INBOUND_XFER 1
+#define OUTBOUND_XFER 2
+#define REQHP_OFF 0x0 /* we read this */
+#define REQTP_OFF 0x4 /* we write this */
+#define RSPHP_OFF 0x8 /* we write this */
+#define RSPTP_OFF 0xc /* we read this */
+
+#define ENCODE_SEM(val, index, sync, cmd, flags) \
+ ((val) | \
+ (index) << 16 | \
+ (sync) << 22 | \
+ (cmd) << 24 | \
+ ((cmd) ? BIT(31) : 0) | \
+ (((flags) & SEM_INSYNCFENCE) ? BIT(30) : 0) | \
+ (((flags) & SEM_OUTSYNCFENCE) ? BIT(29) : 0))
This could be probably better coded using FIELD_PREP()
with integrated checks of passed values not exceed
field width.
Interesting idea. Will have a look.
+struct dbc_req { /* everything must be little endian encoded */
This comment does not provide much value IMHO ...
With the LE types, this does seem to be redundant now. Will remove.
+inline int get_dbc_req_elem_size(void)
+{
+ return sizeof(struct dbc_req);
+}
+
+inline int get_dbc_rsp_elem_size(void)
+{
+ return sizeof(struct dbc_rsp);
+}
.. as those those inliners, instead of using sizeof() directly.
Up to you to change.
Will review these.
+static int reserve_pages(unsigned long start_pfn, unsigned long nr_pages,
+ bool reserve)
+{
+ unsigned long pfn;
+ unsigned long end_pfn = start_pfn + nr_pages;
+ struct page *page;
+
+ for (pfn = start_pfn; pfn < end_pfn; pfn++) {
+ if (!pfn_valid(pfn))
+ return -EINVAL;
+ page = pfn_to_page(pfn);
+ if (reserve)
+ SetPageReserved(page);
+ else
+ ClearPageReserved(page);
It is needed? Looks like taken from some legacy code.
Required for remap_pfn_range().
+static int copy_sgt(struct qaic_device *qdev, struct sg_table **sgt_out,
+ struct sg_table *sgt_in, u64 size, u64 offset)
+{
+ int total_len, len, nents, offf = 0, offl = 0;
+ struct scatterlist *sg, *sgn, *sgf, *sgl;
+ struct sg_table *sgt;
+ int ret, j;
+
+ /* find out number of relevant nents needed for this mem */
+ total_len = 0;
+ sgf = NULL;
+ sgl = NULL;
+ nents = 0;
+
+ size = size ? size : PAGE_SIZE;
+ for (sg = sgt_in->sgl; sg; sg = sg_next(sg)) {
+ len = sg_dma_len(sg);
+
+ if (!len)
+ continue;
+ if (offset >= total_len && offset < total_len + len) {
+ sgf = sg;
+ offf = offset - total_len;
+ }
+ if (sgf)
+ nents++;
+ if (offset + size >= total_len &&
+ offset + size <= total_len + len) {
+ sgl = sg;
+ offl = offset + size - total_len;
+ break;
+ }
+ total_len += len;
+ }
+
+ if (!sgf || !sgl) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
+ if (ret)
+ goto free_sgt;
+
+ /* copy relevant sg node and fix page and length */
+ sgn = sgf;
+ for_each_sgtable_sg(sgt, sg, j) {
+ memcpy(sg, sgn, sizeof(*sg));
+ if (sgn == sgf) {
+ sg_dma_address(sg) += offf;
+ sg_dma_len(sg) -= offf;
+ sg_set_page(sg, sg_page(sgn),
+ sg_dma_len(sg), offf);
+ } else {
+ offf = 0;
+ }
+ if (sgn == sgl) {
+ sg_dma_len(sg) = offl - offf;
+ sg_set_page(sg, sg_page(sgn),
+ offl - offf, offf);
+ sg_mark_end(sg);
+ break;
+ }
+ sgn = sg_next(sgn);
+ }
Why not use sg_copy_table() ? Overall copy_sgt() seems to be something
that could be replaced by generic helper or at least simplify.
I don't see "sg_copy_table" defined in 6.2. Are you suggesting renaming
this function? I guess I'm not quite understanding your comment here.
Can you elaborate?